-
Notifications
You must be signed in to change notification settings - Fork 79
Fixes issue #69 : Enhanced feedback to Selection/Deselection of CheckBox #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request enhances the user experience by providing visual feedback when checkboxes are selected or deselected. It modifies the JavaScript code to toggle CSS classes on the associated label elements, and it adds new CSS classes to define the styles for the selected and deselected states. A transition effect was added for a smoother user experience. Updated class diagram for label elementsclassDiagram
class HTMLLabelElement {
+string className
+add(string className)
+remove(string className)
}
note for HTMLLabelElement "Added 'selectedLabel' and 'unselectedLabel' classes to provide visual feedback"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Preeti9764 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a more descriptive class name than
selectedLabel
andunselectedLabel
. - It looks like the
handleOpenLabelChange
function is setting the same value for bothshowOpenLabel
andshowClosedLabel
- is that intentional?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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.
@@ -82,19 +82,25 @@ function handleEndingDateChange() { | |||
} | |||
function handleLastWeekContributionChange() { | |||
var value = lastWeekContributionElement.checked; | |||
var labelElement = document.querySelector("label[for='lastWeekContribution']"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider creating a helper function to toggle the label classes to reduce code duplication in the handleLastWeekContributionChange and handleOpenLabelChange functions
You can reduce duplication by extracting the label toggling logic into a helper function. For example:
function updateLabelClass(labelElement, isSelected) {
labelElement.classList.toggle("selectedLabel", isSelected);
labelElement.classList.toggle("unselectedLabel", !isSelected);
}
Then update your handlers to use the helper:
function handleLastWeekContributionChange() {
var value = lastWeekContributionElement.checked;
var labelElement = document.querySelector("label[for='lastWeekContribution']");
if (value) {
startingDateElement.disabled = true;
endingDateElement.disabled = true;
endingDateElement.value = getToday();
startingDateElement.value = getLastWeek();
updateLabelClass(labelElement, true);
handleEndingDateChange();
handleStartingDateChange();
} else {
startingDateElement.disabled = false;
endingDateElement.disabled = false;
updateLabelClass(labelElement, false);
}
chrome.storage.local.set({ lastWeekContribution: value });
}
function handleOpenLabelChange() {
var value = showOpenLabelElement.checked;
var labelElement = document.querySelector("label[for='showOpenLabel']");
updateLabelClass(labelElement, value);
chrome.storage.local.set({ showOpenLabel: value });
}
This removes duplicated code while keeping functionality intact.
@hongquan hey can you review my PR and guide me on any further changes required, I find this project really interesting and would love to contribute further. |
Fixes issue #69
Changes:
Summary by Sourcery
Enhance user feedback for checkbox interactions by adding visual styling to improve UI responsiveness
Bug Fixes:
Enhancements: