Conversation
…has resolver errors
…ccess dialog component
… to content' option to modal component
|
Visit the preview URL for this PR (updated for commit e8fc166): https://meemli-dev--pr40-refactor-section-bac-ncc7i78p.web.app (expires Sat, 21 Mar 2026 01:51:41 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 633a893569aee98f762f025e27aa080a469727e1 |
mraysu
left a comment
There was a problem hiding this comment.
Functionality wise, great work. The forms work really well, and I appreciate the modular file structure and compatibility with react form hooks.
Please address the code quality comments below
thomas-rocha
left a comment
There was a problem hiding this comment.
Looks pretty good, I have one concern about functionality and a few about styling.
No error messages appear for the start and end-time if they are not filled out when trying to proceed.

The error message for the day selection appears to the right instead of underneath which throws off the UI imo since the rest gets pushed to the right.

Selecting many days expands the main bar to contain all of the entries at once, which messes up the layout. I'd recommend fixing the width, hiding excess elements on overflow, and allowing the user to scroll through the main bar to look at all the entries.

This one might be a little nitpicky, but the user has to uncheck a selected time before they can select a new one if they misclick. This feels a bit clunky imo, so maybe allowing them to switch to whichever one they click automatically could work out.

Not sure if this was intended or not, but I thought I'd point it out. The cancel button when creating a class resets the progress, but clicking outside the modal doesn't.
Other than these, the code looks good to me.


Tracking Info
Resolves #30
Changes
startDate: string
endDate: string
archived: boolean, default: False
color: string
Testing
Checked Mongodb Compass app
Confirmation of Change