Schedule changes #310
Schedule changes #310
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
IllGive
added
feature improvement
quick007
left a comment
quick007
left a comment
There was a problem hiding this comment.
Few things:
First off, we have a pr template for a reason. it makes it significantly harder to review your code and harder to catch things that were changed, especially when I have to go back and ask you after them in a later pr.
when set to 0 schedules, the classes should extend across
didn't catch this in ur last pr, but please change the way these these dates are displayed here:
that overflow looks really weird, and the calendar day isn't all that important for a user. What if you did this?
also btw idk if this was actually finished, but please make this a draft if it wasn't
| <DropdownSection | ||
| name="Number of schedules to show" | ||
| description="Choose how many days of schedules to show on your homepage. By default we show two." | ||
| currentValue={ | ||
| numberOfSchedulesToShow.find( | ||
| (theNumber) => theNumber.id == settings.schedulesToShow | ||
| )! | ||
| } | ||
| values={numberOfSchedulesToShow} | ||
| onChange={(value) => | ||
| set({ schedulesToShow: value.id as Settings["schedulesToShow"] }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
for some reason the dropdown section isn't always to the right
| homepageAssignments: "all" | "student" | "none"; | ||
| homepageView: "auto" | "tabbed" | "student" | "teacher"; | ||
| showAMPM: boolean; | ||
| schedulesToShow: "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7"; |
There was a problem hiding this comment.
for future reference, it might be easier to use an enum or normal numbers, but this works just as well.
There was a problem hiding this comment.
tried that, turns out that settings wanted strings and I was too tired to deal with it
| {schedules.length > 2 && !showMoreSchedules && ( | ||
| <Button | ||
| className="justify-center max-lg:hidden" | ||
| onClick={() => setShowMoreSchedules(true)} | ||
| > | ||
| Show More Schedules | ||
| </Button> | ||
| )} | ||
| {showMoreSchedules && ( | ||
| <div className="max-lg:hidden"> | ||
| {schedules.slice(2).map((schedule, index) => ( | ||
| <div key={index} className="w-full md:mr-4 lg:mr-0"> | ||
| <h2 className="title mr-2"> | ||
| {dateFormat.format(schedule.date)} | ||
| </h2> | ||
|
|
||
| <ScheduleComponent | ||
| classes={classes} | ||
| schedule={schedule.schedule} | ||
| loading={loading} | ||
| /> | ||
| </div> | ||
| ))} | ||
| <Button | ||
| className="justify-center" | ||
| onClick={() => setShowMoreSchedules(false)} | ||
| > | ||
| Show Fewer Schedules | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Multiple issues with this button:
- it disappears on smaller screen sizes
- if you select more than 2 schedules in settings, this button appears. If I'm a student and want to see 3 schedules on my homescreen, why would I want to have to click this every single time
let's remove this for now, or at least find a better way to implement it
There was a problem hiding this comment.
I thought that's what you wanted....
k. should have been draft though, ur right
There was a problem hiding this comment.
oh, my bad. probably should've discussed this one more
| <section | ||
| id="Schedule" | ||
| className="flex grow flex-col md:flex-row lg:ml-10 lg:flex-col" | ||
| className="flex grow flex-col md:flex-row lg:ml-10 lg:flex-col max-lg:" |
There was a problem hiding this comment.
why did you add max-lg here
See linked issue
Added setting to choose how many schedules to show on the homepage, and added a show more schedule button