DMC/studentportalen
1
1
Fork 0
Code Issues 50 Pull Requests Actions Projects 1 Wiki Activity

Start time grid #62

Merged
stne3960 merged 30 commits from start_time_grid into main 2026-01-16 14:17:09 +01:00
Conversation 7 Commits 30 Files Changed 32 +2147 -464
stne3960 commented 2026-01-09 11:46:47 +01:00
Owner
Copy Link

This PR addresses issues #23 #24 #25 and #26 and rearranges the component library

This PR addresses issues #23 #24 #25 and #26 and rearranges the component library
stne3960 added 18 commits 2026-01-09 11:46:49 +01:00
Add StartTimeGrid
All checks were successful
Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 3m46s
Details
10b4a39043
gitea-actions commented 2026-01-09 11:50:34 +01:00
First-time contributor
Copy Link

Deployed to https://studentportalen-starttimegrid.branch.dsv.su.se

Deployed to https://studentportalen-starttimegrid.branch.dsv.su.se
stne3960 added the
Code Review
Design Review
 labels 2026-01-09 11:52:08 +01:00
stne3960 requested review from ansv7779 2026-01-09 11:52:28 +01:00
stne3960 requested review from jare2473 2026-01-09 11:52:29 +01:00
jare2473 requested changes 2026-01-09 14:28:53 +01:00
Dismissed
jare2473 left a comment
Owner
Copy Link

Design-related to be fixed

Inline modal

  • Background color should be Sky/20 (my bad!)
  • “Room” label is not correct font and style

image.png

Choicebox

  • Unavailable state should not display a radio button circle. Or did you have a reason for keeping the radio button circle?

Start time grid

  • Change margin top for inline modal to Spacing/Ml
  • No “Upp till...” text on 30 minute slots. Should just say “30 minuter”
  • Text style on “Visa lediga tider” is Body/Light/Md

Grid keyboard navigation

Not a dealbreaker; can be improved later.

  • Keyboard navigation should follow a grid pattern (similar to https://react-aria.adobe.com/GridList). Arrow keys should move based on visual position, e.g. pressing ↓ on 09:00 should move to 10:00, not 09:30.
  • Users should also be able to navigate the grid without the inline modal opening automatically.
# Design-related to be fixed ## Inline modal - [x] Background color should be Sky/20 (my bad!) - [x] “Room” label is not correct font and style ![image.png](/attachments/9dca97ce-1339-494f-8d73-a21050bda645) ## Choicebox - [x] Unavailable state should not display a radio button circle. *Or did you have a reason for keeping the radio button circle?* ## Start time grid - [x] Change margin top for inline modal to Spacing/Ml - [x] No “Upp till...” text on 30 minute slots. Should just say “30 minuter” - [x] Text style on “Visa lediga tider” is Body/Light/Md ### Grid keyboard navigation Not a dealbreaker; can be improved later. - [ ] Keyboard navigation should follow a grid pattern (similar to https://react-aria.adobe.com/GridList). Arrow keys should move based on visual position, e.g. pressing ↓ on 09:00 should move to 10:00, not 09:30. - [ ] Users should also be able to navigate the grid without the inline modal opening automatically.
image.png
60 KiB
stne3960 added 7 commits 2026-01-09 15:19:29 +01:00
npm format
All checks were successful
Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 3m40s
Details
79778e02fe
stne3960 commented 2026-01-09 15:25:14 +01:00
Author
Owner
Copy Link

Grid keyboard navigation

Not a dealbreaker; can be improved later.

  • Keyboard navigation should follow a grid pattern (similar to https://react-aria.adobe.com/GridList). Arrow keys should move based on visual position, e.g. pressing ↓ on 09:00 should move to 10:00, not 09:30.
  • Users should also be able to navigate the grid without the inline modal opening automatically.

I fixed the other parts, but this should be it's own separate issue, it requires a bit of thought and work. Like selection behavior, once you press Enter to select a slot and the modal opens, should arrow keys continue navigating the grid (closing/moving the modal?) or be trapped in the modal until it's dismissed? Should Escape close the modal and return focus to the grid?

> > ### Grid keyboard navigation > > Not a dealbreaker; can be improved later. > > - [ ] Keyboard navigation should follow a grid pattern (similar to https://react-aria.adobe.com/GridList). Arrow keys should move based on visual position, e.g. pressing ↓ on 09:00 should move to 10:00, not 09:30. > - [ ] Users should also be able to navigate the grid without the inline modal opening automatically. I fixed the other parts, but this should be it's own separate issue, it requires a bit of thought and work. Like selection behavior, once you press Enter to select a slot and the modal opens, should arrow keys continue navigating the grid (closing/moving the modal?) or be trapped in the modal until it's dismissed? Should Escape close the modal and return focus to the grid?
jare2473 approved these changes 2026-01-09 15:56:26 +01:00
jare2473 left a comment
Owner
Copy Link

Looks good! Just a tiny detail to update the arrow background color to Sky/20 as well

Screenshot 2026-01-09 at 15.53.03.png
Looks good! Just a tiny detail to update the arrow background color to Sky/20 as well <img width="175" alt="Screenshot 2026-01-09 at 15.53.03.png" src="attachments/9d1678f9-c687-465e-8dab-b1d1348b1374">
Screenshot 2026-01-09 at 15.53.03.png
7.0 KiB
👍 1
stne3960 added 1 commit 2026-01-09 16:13:27 +01:00
Change background on arrow
All checks were successful
Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 3m41s
Details
29fdbd5146
ansv7779 requested changes 2026-01-12 12:07:16 +01:00
Dismissed
ansv7779 left a comment
Owner
Copy Link

Use the new Temporal API with one of the available polyfills (temporal-polifyll being the most popular). Having everything as a string and using regular expressions or string splitting is not good.

Use the new [Temporal API](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal) with one of the available polyfills ([temporal-polifyll](https://www.npmjs.com/package/temporal-polyfill) being the most popular). Having everything as a `string` and using regular expressions or string splitting is not good.
frontend/src/components/InlineModal/InlineModal.tsx Outdated
@ -0,0 +27,4 @@
right: "right-[58px]",
};
export default function InlineModal({
ansv7779 commented 2026-01-12 11:48:27 +01:00
Owner
Copy Link

This needs some documentation what it is. Inline and modal are to me opposing concepts. What actually is it? When would I use it compared to a modal dialog and how is it different from just a <div>?

This needs some documentation what it is. Inline and modal are to me opposing concepts. What actually is it? When would I use it compared to a modal dialog and how is it different from just a `<div>`?
stne3960 marked this conversation as resolved
frontend/src/components/InlineModal/InlineModal.tsx
@ -0,0 +38,4 @@
return (
<div className={clsx(baseClasses, className)} {...props}>
{/* Arrow pointing up - uses two layered CSS triangles to create a bordered arrow effect.
ansv7779 commented 2026-01-12 11:49:10 +01:00
Owner
Copy Link

Everything else is a SVG icon, why not this too? Using a CSS border trick feels like a relic from the past.

Everything else is a SVG icon, why not this too? Using a CSS border trick feels like a relic from the past.
stne3960 marked this conversation as resolved
stne3960 removed the
Design Review
 label 2026-01-12 18:32:11 +01:00
stne3960 added 3 commits 2026-01-13 08:04:27 +01:00
Use temporal-polyfill instead of string manipulation
All checks were successful
Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 4m17s
Details
0fc2f7c0d8
stne3960 requested review from ansv7779 2026-01-13 08:42:20 +01:00
ansv7779 requested changes 2026-01-14 10:39:04 +01:00
Dismissed
ansv7779 left a comment
Owner
Copy Link

Everything is still typed as string and continuously parsed to temporal types (that get immediately thrown away). The useGroupRoomBooking should convert the stringly typed JSON object from the API to well-typed data for use in the rest of the system.

Taking a string, converting to PlainDateTime, converting to PlainTime, converting to string, only to then convert back to PlainTime is not ideal.

function timeToMinutes(time: string): number {
  const t = Temporal.PlainTime.from(time);
  return t.hour * 60 + t.minute;
}

const bookingDateTime = Temporal.PlainDateTime.from(booking.start);
const bookingStart = timeToMinutes(bookingDateTime.toPlainTime().toString());
Everything is still typed as `string` and continuously parsed to temporal types (that get immediately thrown away). The `useGroupRoomBooking` should convert the stringly typed JSON object from the API to well-typed data for use in the rest of the system. Taking a `string`, converting to `PlainDateTime`, converting to `PlainTime`, converting to string, only to then convert back to `PlainTime` is not ideal. ```typescript function timeToMinutes(time: string): number { const t = Temporal.PlainTime.from(time); return t.hour * 60 + t.minute; } const bookingDateTime = Temporal.PlainDateTime.from(booking.start); const bookingStart = timeToMinutes(bookingDateTime.toPlainTime().toString()); ```
stne3960 added 1 commit 2026-01-14 23:40:00 +01:00
Refactor booking components to use Temporal types internally with string conversion at system boundaries
All checks were successful
Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 3m34s
Details
Remove branch deployment from branch.dsv.su.se / cleanup (pull_request) Successful in 10s
Details
89f3ba2dec
stne3960 requested review from ansv7779 2026-01-14 23:47:44 +01:00
ansv7779 approved these changes 2026-01-16 13:39:02 +01:00
stne3960 merged commit e1029f52a5 into main 2026-01-16 14:17:09 +01:00
stne3960 referenced this issue from a commit 2026-01-16 14:17:10 +01:00
Start time grid (#62)
stne3960 deleted branch start_time_grid 2026-01-16 14:17:14 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
jare2473
ansv7779
Labels
Clear labels
Code Review
Design
Component
Design
Page
Design Review
Design in progress
Design system
Front end
bug
Something is not working
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
priority
question
More information is needed
wontfix
This won't be fixed
No Label
Code Review
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DMC/studentportalen#62
No description provided.
Delete Branch "start_time_grid"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?