Refactor dark mode implementation and improve styling consistency#350
Refactor dark mode implementation and improve styling consistency#350SRSoham wants to merge 3 commits into
Conversation
|
@SRSoham is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @komalharshita, Key updates:
Tested locally for theme switching, responsiveness, and UI consistency across major sections. |
komalharshita
left a comment
There was a problem hiding this comment.
This is a strong visual improvement overall — the dark mode coverage is much more complete now, and several UI areas feel significantly more polished and consistent.
Things done well:
- Expanded dark mode styling across many previously inconsistent components
- Good use of CSS variables
- Clean localStorage toggle implementation
- Marquee layout cleanup improves stability
- Better visual consistency across cards/buttons/pills/modals
However, a few important issues should be addressed before merge:
-
package-lock.jsonchanges should be removed unless actual dependency updates are required. -
The theme toggle button is missing accessibility attributes like:
aria-labelaria-pressed
-
No reduced-motion handling was added for animations/transitions.
-
The implementation currently relies on many repeated component-specific dark mode overrides, which may become difficult to maintain long-term.
-
Multiple
!importantdeclarations suggest CSS specificity issues that should ideally be resolved architecturally instead. -
Theme initialization likely causes a flash-of-incorrect-theme (FOUC) before dark mode loads.
-
prefers-color-schemesupport is currently missing for first-time users.
Overall, the UI work is genuinely good, but the accessibility and maintainability concerns should be resolved before approval.
|
Hi @komalharshita , thank you for the detailed review and suggestions. I’ve addressed the requested accessibility improvements, reduced-motion support, and resolved the merge conflicts. Kindly review the updated changes when you get time. Thank you! |
|
Hi @komalharshita , just following up on this PR. I’ve addressed the requested review changes and resolved the merge conflicts. Kindly review it when you get time. Thank you! |
komalharshita
left a comment
There was a problem hiding this comment.
Thanks for addressing the accessibility concerns, reduced-motion support, and resolving the merge conflicts.
The dark mode implementation looks much stronger now. Before approval, I'd like one final verification:
- Please confirm the new skill-strip marquee structure does not introduce duplicate content or layout issues across screen sizes.
- Verify theme persistence works correctly (save/restore state, icon updates, and aria attributes).
Once those checks are confirmed and the preview looks good, this should be ready for merge.
If the Vercel preview and local testing look good, I'd be comfortable approving this PR after that final check.
|
Hi @komalharshita, I completed the final verification checks locally.
The local preview looks stable now. Thank you for the review! |
|
Hi @komalharshita, I verified the latest working state locally:
The current branch is working correctly locally. Since the main branch has moved ahead again, there are merge conflicts showing on GitHub. Please let me know if you’d prefer me to resolve them again locally or if you’d like to handle the final rebase/merge from your side. Thank you! |
|
@SRSoham you will need to resolve them locally, as there are several contributors who are working for the similar codespace directory |
|
@komalharshita Due to repeated merge conflicts after overlapping updates landed in the same sections, I rebuilt the implementation cleanly on top of the latest main branch and opened a fresh PR with the resolved version. The new PR includes:
Continuing review/discussion in the new PR. |
|
Closing this PR as the updated one has been merged |
Changes Made
This PR refactors and improves the previous dark mode implementation after review feedback from the earlier PR (#248).
Improvements
[data-theme="dark"]component-based overrides!importantusageUpdated Components
Fixes
Previous PR
This PR continues and refines the earlier dark mode implementation:
Testing
Tested locally for: