Conversation
…yboard space/enter
| &:focus-within { | ||
| outline: 2px solid ${Colour["focus-ring"]}; | ||
| outline-offset: 1px; | ||
| z-index: 1; |
| const cellsArray = getCellsForDate(formattedDate); | ||
| return ( | ||
| <TimeSlotWrapper key={`wrapper-${dayIndex}`}> | ||
| {cellsArray.map((slot) => { |
There was a problem hiding this comment.
the nesting is kind of hard to read, let's move the rendering of TimeSlotWrapper to a new renderSlotCell function
| aria-disabled={ | ||
| !clickable | ||
| } | ||
| aria-describedby={ |
There was a problem hiding this comment.
why aria-describedby instead of as part of aria-label?
| backgroundColor, | ||
| backgroundColor2, | ||
| } = styleAttributes; | ||
| const slotKey = `${formattedDate}-${id}`; |
There was a problem hiding this comment.
| const slotKey = `${formattedDate}-${id}`; | |
| const slotId = `${formattedDate}-${id}`; |
| } | ||
| ) | ||
| } | ||
| onClick={handleSlotButtonClick( |
There was a problem hiding this comment.
hmm thinking of inlining like this so that the default assignment of clickable is kept in one place
| onClick={handleSlotButtonClick( | |
| onClick={clickable && handleSlotButtonClick( |
| startTime = "", | ||
| endTime = "" |
There was a problem hiding this comment.
why add the additional arguments?
| setExpandAll((prevExpandValue) => !prevExpandValue); | ||
| }; | ||
|
|
||
| const getCellsForDate = (formattedDate: string) => { |
There was a problem hiding this comment.
this should sit under the helper functions section, not the event handlers. you can hoist it if needed using non-arrow functions
| const getFocusableSlotsForDate = ( | ||
| formattedDate: string | ||
| ): FocusableSlotMeta[] => { | ||
| return getCellsForDate(formattedDate) |
There was a problem hiding this comment.
we'll be re-computing this for every key down event, suggest to compute for all dates once and memoise since both rendering and event handling need this
| const hasCollapsedContent = | ||
| !!maxVisibleCellHeight && | ||
| actualHeight > maxVisibleCellHeight; |
There was a problem hiding this comment.
looks like we're doing the same checks to determine if content is collapsed, as rendering, can pull this into a shared variable or function
There was a problem hiding this comment.
this can happen. we should probably set inert on the elements that are after the visible height
Screen.Recording.2026-04-15.at.8.18.40.PM.mov
2 participants
Type of changes
Description of changes
Checklist
Looks good on mobile and tablet
Screen reader annotates as per figma
Link to Figma