feat: Reusable function appendPUSCheckbox #108
Conversation
DanieliusAsm
force-pushed
the
more_improvements
branch
from
0b95479 to
0b4142b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a reusable function appendPUSCheckbox to standardize the creation of checkbox elements throughout the codebase, replacing hardcoded HTML strings with function calls.
- Creates a centralized function for generating checkbox HTML markup
- Refactors existing hardcoded checkbox HTML to use the new function
- Adds a new checkbox option for "Image rotation and Hunger Games buttons"
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ((pageType === "edit") ? | ||
| '<li><input class="pus-checkbox" type="checkbox" id="pus-helpers" checked><label for="pus-helpers">Field helpers</label></li>' + | ||
| '<li><input class="pus-checkbox" type="checkbox" id="pus-dist-free"><label for="pus-dist-free">Distraction free mode</label></li>': | ||
| appendPUSCheckbox('pus-helpers','Field helpers')+''+ |
There was a problem hiding this comment.
The empty string concatenation +'' is unnecessary and adds visual clutter. Consider removing it for cleaner code.
| appendPUSCheckbox('pus-helpers','Field helpers')+''+ | |
| appendPUSCheckbox('pus-helpers','Field helpers')+ |
|
|
||
| } // if list mode | ||
|
|
||
| function appendPUSCheckbox(checkboxId, labelText){ |
There was a problem hiding this comment.
The function is defined after it's used in the code above (lines 756, 757, 760, 1189-1191). Consider moving the function definition before its first usage to improve code readability and avoid potential hoisting confusion.
| }); | ||
|
|
||
| } // if list mode | ||
|
|
There was a problem hiding this comment.
The function lacks documentation. Consider adding a JSDoc comment to describe its purpose, parameters, and return value for better maintainability.
| /** | |
| * Returns an HTML string for a checkbox with a label, used in the Power User Script UI. | |
| * @param {string} checkboxId - The id attribute for the checkbox input and its label. | |
| * @param {string} labelText - The text to display in the label. | |
| * @returns {string} HTML string for a list item containing the checkbox and label. | |
| */ |
| function appendPUSCheckbox(checkboxId, labelText){ | ||
| return `<li><input class="pus-checkbox" type="checkbox" id="${checkboxId}"><label for="${checkboxId}">${labelText}</label></li>`; |
There was a problem hiding this comment.
The function doesn't validate or escape the input parameters. If checkboxId or labelText contain HTML special characters, they could lead to XSS vulnerabilities. Consider adding input validation or HTML escaping.
| function appendPUSCheckbox(checkboxId, labelText){ | |
| return `<li><input class="pus-checkbox" type="checkbox" id="${checkboxId}"><label for="${checkboxId}">${labelText}</label></li>`; | |
| function escapeHtml(str) { | |
| return String(str) | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, '''); | |
| } | |
| function appendPUSCheckbox(checkboxId, labelText){ | |
| const safeId = escapeHtml(checkboxId); | |
| const safeLabel = escapeHtml(labelText); | |
| return `<li><input class="pus-checkbox" type="checkbox" id="${safeId}"><label for="${safeId}">${safeLabel}</label></li>`; |
What