ic-ui-kit icon indicating copy to clipboard operation
ic-ui-kit copied to clipboard

3671 time selector component

Open GCHQ-Developer-530 opened this issue 3 months ago • 9 comments

Summary of the changes

Implement time selector component.

NOTE: The column focus indicator doesn't quite match the Figma designs, this is because matching the designs meant that items weren't clickable when the column was focused. We've decided this different style is okay for now as it's canary, and we can look at it further down the line if it's an issue.

Related issue

#3671

Checklist

General

  • [x] Changes to docs package checked and committed.
  • [x] All acceptance criteria reviewed and met.

Testing

  • [x] Relevant unit tests and visual regression tests added.
  • [x] Visual testing against Figma component specification completed.
  • [x] Playground stories in React Storybook up to date, with any prop changes and additions addressed.

Accessibility

  • [x] Accessibility Insights FastPass performed.
  • [x] A11y unit test added and yields no issues.
  • [x] A11y plug-in on Storybook yields no issues.
  • [x] Manual screen reader testing performed using NVDA and VoiceOver.
  • [x] Manual keyboard testing for keyboard controls and logical focus order.
  • [x] Correct roles used and ARIA attributes used correctly where required.

Resize/zoom behaviour

  • [x] Page can be zoomed to 400% with no loss of content.
  • [x] Screen magnifier used with no issues.
  • [x] Text resized to 200% with no loss of content.
  • [x] Text spacing increased as per the WCAG 1.4.12 success criterion with no loss of content.

System modes

  • [x] Browser setting 'prefers reduced motion' tested. No animations or motion visible whilst this setting is on.
  • [x] Windows High Contrast mode tested with no loss of content.
  • [x] System light and dark mode tested with no loss of content.
  • [x] Browser support tested (Chrome, Safari, Firefox and Edge).

Testing content extremes

  • [x] Min/max content examples tested with no loss of content or overflow.
  • [x] All prop combinations work without issue.
  • [x] Tested for FOUC (Flash of Unstyled Content) in both SSR (Server-Side Rendering) and SSG (Static Site Generation) settings.
  • [x] Props/slots can be updated after initial render.

GCHQ-Developer-530 avatar Oct 07 '25 09:10 GCHQ-Developer-530

View your branch deployment here: https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/web-components View your React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/react

github-actions[bot] avatar Oct 07 '25 09:10 github-actions[bot]

View your canary branch deployment here: https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/canary-web-components View your canary React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/canary-react

github-actions[bot] avatar Oct 07 '25 09:10 github-actions[bot]

Created #4018 for adding pageup/down functionality.

GCHQ-Developer-718 avatar Oct 13 '25 12:10 GCHQ-Developer-718

Cypress visual tests failed. View the image diff here: https://github.com/mi6/ic-ui-kit/tree/gh-pages/branches/3671-time-selector-component/cypress-image-diff-screenshots/diff View the html report here: https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/cypress-image-diff-html-report/cypress-image-diff-html-report.html

github-actions[bot] avatar Oct 13 '25 13:10 github-actions[bot]

selected items styling gets a little funky when changing zoom level, eg 110%:

image

ad9242 avatar Oct 23 '25 12:10 ad9242

I can't seem to interact with the scrollable lists when using NVDA. ArrowUp & Down do not work. Normally you can use "Alt + ArrowUp / ArrowDown", but that does not seem to work. using Alt + ArrowUp/Down works ok in Datepicker

ad9242 avatar Oct 23 '25 12:10 ad9242

i have a gripe with the am\pm selector - probably one to go back to design. I don't like the way the items move around as you select one. i feel it would be better if it used inline toggle buttons. something like this:

image

EDIT: maybe this is just a personal thing. After looking around the common approach seems to be to have the am\pm stacked vertically

ad9242 avatar Oct 23 '25 14:10 ad9242

selected items styling gets a little funky when changing zoom level, eg 110%:

image

I think we decided that this isn't too much of an issue as a similar thing happens on radio, but happy to look into it further if people think it's a bigger issue

GCHQ-Developer-530 avatar Oct 30 '25 11:10 GCHQ-Developer-530

https://github.com/user-attachments/assets/ab1a57e7-4cb0-40b0-aa26-62c7a89c7b35

Noticed a bit of weird behaviour when using Voiceover.

  • There doesn't seem to be a way to hear the whole currently selected time
  • When moving between fields, it tends to read the value of the previous field and it reads it twice.

Unrelated feedback:

  • Being able to scroll the fields is cool, feels weird that it doesn't select the value that you land on. I've gotten used to the scrollwheels on a COTS timekeeping app tho.
  • I was assuming that I could, when focussed on a field, type numbers to set the value. E.g. typing 1, 2 and the value would be set to 12. Like how Selects work. This might be a job for a separate ticket or for forgetting about until a user asks for it

GCHQ-Developer-299 avatar Nov 21 '25 15:11 GCHQ-Developer-299

https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/canary-web-components

I think I've managed to fix the issue of it not reading the whole selected time, and reading out the previous field when moving between fields.

I will open a ticket for adding the typing numbers functionality as I think it could be quite involved working out wait times and deciding if 5 and then 8 is 58 or they want to switch between 5 and 8.

GCHQ-Developer-530 avatar Nov 28 '25 11:11 GCHQ-Developer-530

https://mi6.github.io/ic-ui-kit/branches/3671-time-selector-component/canary-web-components

I think I've managed to fix the issue of it not reading the whole selected time, and reading out the previous field when moving between fields.

I will open a ticket for adding the typing numbers functionality as I think it could be quite involved working out wait times and deciding if 5 and then 8 is 58 or they want to switch between 5 and 8.

I'm still getting it read the Selected hour/minute/second out twice when I move between values - but it does read them all okay :)

GCHQ-Developer-299 avatar Nov 28 '25 12:11 GCHQ-Developer-299