Sharadha taking over for Manvitha- Phase 4 – Timer Functionality#4432
Sharadha taking over for Manvitha- Phase 4 – Timer Functionality#4432myeeli wants to merge 27 commits into
Conversation
…tegrated with backend
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
myeeli
changed the title
one-community
added
the
High Priority - Please Review First
Anusha-Gali
left a comment
Anusha-Gali
left a comment
There was a problem hiding this comment.
Hi Manvitha,
I have reviewed your PR locally and have posted my comments on the frontend pr: OneCommunityGlobal/HGNRest#1916 (review)
VijayAnirudh
left a comment
VijayAnirudh
left a comment
There was a problem hiding this comment.
Hello!
- Functionality works.
- Font color made the text camouflaged (my browser rendered it differently, I'm assuming!)
- Dark mode isn't supported.
rohanrastogi311
left a comment
rohanrastogi311
left a comment
There was a problem hiding this comment.
Nice implementation.
Few issues: 1) Point 4 on your "How to test" is an incorrect URL.
2) Dark Mode functionality isn't implemented well.
3) The seconds has no up or down arrows and neither functionality respectively.
|
SharadhaKasiviswanathan
changed the title
- Remove unused imports (KeyboardArrowUpRoundedIcon, KeyboardArrowDownRoundedIcon) - Remove unused msToHMS function - Wrap event handlers and API helpers in useCallback - Replace empty catch blocks with error state updates - Add complete popup content (time editor grid, seconds up/down, quick presets, progress circle, controls) - Add seconds state with increment/decrement and input - Add dark mode font colors for colon separator in timer.module.css - Remove broken :global(.dark-mode) CSS nesting from StudentDashboard.module.css
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
- TaskTimer.jsx: add onKeyDown to backdrop for keyboard a11y - TaskTimer.jsx: use optional chaining for optional chain rule - TaskTimer.jsx: remove unused loading state variable - timer.module.css: darken #3b82f6 to #2563eb for WCAG AA contrast - StudentDashboard.module.css: same contrast fix, remove commented code - StudentDashboard.jsx: replace empty catch blocks with console.error
Dismissing review as the feedback on this PR and the associated backend PR (#1916) has been resolved. The quality gate and ESLint accessibility warnings have also been cleared.
|
There was a problem hiding this comment.
Hi Sharadha,
I ran and tested the PR locally alongside the associated backend.
Verfied
-
"-" and "+" button are responsive and work as intended
-
Dark Mode looks fine
Issues
- Can't seem to start and reset the timer. Getting blocked by CORs policy
-
Note: I can't test the stop button either since the timer is never started so I can't verify the behavior
-
The pop up timer component causes the screen to grey out and it seems like it's not clickable as clicking anywhere causes the component to disappear so the component cannot be verified. And it's not centered
HemanthNidamanuru
left a comment
HemanthNidamanuru
left a comment
There was a problem hiding this comment.
Hi,
I tested this PR locally. The Start and Reset timer buttons are not working, so I was also unable to verify the Stop button functionality.
Additionally, the timer popup greys out the screen and disappears when clicking anywhere, which prevents proper testing. The popup is also not centered on the page.
one-community
added
Needs New Developer
8 participants
Description
Related PRS (if any):
This frontend PR is related to the #1916 backend PR.
…
Main changes explained:
…
How to test:
npm installand...to run this PR locallyScreenshots or videos of changes:
1.mp4
Note:
Include the information the reviewers need to know.