-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add classroom timepicker #612
Conversation
Visit the preview URL for this PR (updated for commit 670a3b5): https://jump-math-staging--pr612-juliands-add-classro-td0l3hio.web.app (expires Tue, 28 Nov 2023 00:53:39 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c42d8d0d853b05885664a2dd73f8245f4333ae51 |
@@ -34,6 +34,9 @@ const chakraStyles = ( | |||
outline: "none", | |||
}, | |||
border: "none", | |||
_disabled: { | |||
opacity: 1, |
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.
what's this for?
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.
Due to an implementation detail the opacity
-based styling for disabled elements was getting applied twice, meaning that this component looked different than other disabled components. Technically this fix belongs in the previous pr but that's already merged so I thought this was fine.
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.
looks good (i changed base branch for easier review - you can revert)
Placeholders are sentences, so they should read like one instead of like a title. For example, you probably wouldn't use "Please choose a date" as a title.
b720768
to
670a3b5
Compare
(force push just to rebase on previous PR) |
Notion ticket link
Add start / end time in classrooms
Implementation description
Update the date picker in the classroom modal to be a DateTimePicker. This PR also includes a bunch of minor UI fixes related to this change; have a look at the commits for a full list.
Steps to test
What should reviewers focus on?
Checklist