feature: allow users to paste a multi line string into the options #223
feature: allow users to paste a multi line string into the options #223
Conversation
…eld and convert it into multiple bullet points
Reviewer's Guide by SourceryThis PR enhances the poll options functionality by enabling users to paste multi-line strings, which are then split into multiple options. It implements a paste handler to process the pasted text and updates the addChoiceField function to accommodate preset values when creating new options. Sequence diagram for multi-line paste handlingsequenceDiagram
actor User
participant "Poll Options Input" as Input
participant "Paste Event Handler" as Handler
participant "addChoiceField" as AddOption
User->>Input: Paste multi-line string
Input->>Handler: Trigger paste event
Handler->>Handler: Retrieve clipboardData
Handler->>Handler: Check if pastedData contains newlines
alt Multi-line detected
Handler->>Handler: Prevent default paste
Handler->>Handler: Split text into lines
Handler->>Input: Set current input value to first line
loop For each additional line
Handler->>AddOption: addChoiceField(line)
end
else Single line
Handler->>Input: Normal paste handling
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add functionality to handle pasted input within the poll options section. When a user pastes text into an option field, the code checks for newline characters. If found, it prevents the default action, inserts the first line into the current input, and creates additional input fields for subsequent lines using the modified Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Input as Input Field
participant Handler as Paste Event Handler
participant Function as addChoiceField()
User->>Input: Paste text with newlines
Input->>Handler: Trigger paste event
Handler->>Handler: Check for newline characters
alt Newlines detected
Handler->>Input: Set first line as current input value
Handler->>Function: For each extra line, call addChoiceField(line)
else No newlines
Handler->>Input: Process default paste behavior
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @fsargent - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a more specific selector to the paste event listener to only target the option inputs you want to enable this feature on.
- It might be good to have a configuration option to disable the multi-line paste functionality.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| pollOptions.container.on("paste", "input[type=text]", function (e) { | ||
| // Get the pasted content | ||
| const pastedData = e.originalEvent.clipboardData.getData("text"); | ||
|
|
||
| // Check if the pasted content contains newlines | ||
| if (pastedData.includes("\n")) { | ||
| e.preventDefault(); // Prevent default paste | ||
|
|
||
| // Split the pasted content into lines | ||
| const lines = pastedData |
There was a problem hiding this comment.
issue: Check for empty pasted lines before setting the value.
If the pasted data consists entirely of newlines or spaces (resulting in an empty array after filtering), then accessing lines[0] may assign undefined to the input value. Consider adding a guard to ensure that there is at least one non-empty line before setting the value and creating additional options.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
approval_polls/staticfiles/create.js (1)
46-66:⚠️ Potential issuePrevent XSS by escaping the value parameter.
The value parameter is inserted directly into the HTML template without escaping, which could lead to XSS attacks.
Add value escaping:
const addChoiceField = (value = "") => { lastId++; + // Escape HTML special characters + const escapedValue = value + .replace(/&/g, "&") + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + const newOption = ` <div class="mb-3 poll-option"> <label for="choice${lastId}" class="form-label">Option ${lastId}</label> <div class="input-group"> <input type="text" class="form-control" id="choice${lastId}" name="choice${lastId}" maxlength="100" placeholder="Option Name" - value="${value}"> + value="${escapedValue}"> <button class="btn btn-outline-primary remove-choice" type="button" title="Remove Choice"> <i class="fa fa-times"></i> </button> </div> </div> `; pollOptions.container.append(newOption); };
🧹 Nitpick comments (1)
approval_polls/staticfiles/create.js (1)
18-41: Consider enhancing the paste handling.The paste handling implementation is good but could be improved in a few ways:
- Consider handling tab-separated values
- Add support for copying from spreadsheets
- Consider adding a maximum limit to prevent too many options
Here's an enhanced implementation:
pollOptions.container.on("paste", "input[type=text]", function (e) { const pastedData = e.originalEvent.clipboardData.getData("text"); - if (pastedData.includes("\n")) { + if (pastedData.includes("\n") || pastedData.includes("\t")) { e.preventDefault(); // Split the pasted content into lines const lines = pastedData - .split("\n") + .split(/[\n\t]/) .map((line) => line.replace(/^(-|•|\*|⁃|◦|▪︎|●|■)+/, "").trim()) .filter((line) => line.length > 0); + // Limit the number of options + const MAX_OPTIONS = 50; + if (lines.length > MAX_OPTIONS) { + alert(`Maximum ${MAX_OPTIONS} options allowed`); + return; + } + // Set the first line in the current input $(this).val(lines[0]); // Create new options for the remaining lines lines.slice(1).forEach((line) => { addChoiceField(line); }); } });🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: A character class cannot match a character and a combining character.
A character and a combining character forms a new character. Replace the character class with an alternation.
(lint/suspicious/noMisleadingCharacterClass)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
approval_polls/staticfiles/create.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
approval_polls/staticfiles/create.js
[error] 30-30: A character class cannot match a character and a combining character.
A character and a combining character forms a new character. Replace the character class with an alternation.
(lint/suspicious/noMisleadingCharacterClass)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: review_app
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
field and convert it into multiple bullet points
Resolves #222
Summary by Sourcery
New Features: