feat(Timepicker): Add combined time column

[elephantcatdog]

Adds useCombinedRenderer and combinedRenderer properties to Timepicker. useCombinedRenderer combines the individual time columns into one column. combinedRenderer allows users to customize the combined column.

Some places I think could use some improvement:

• In the return function I am checking for useCombinedRenderer before each column addition so that only the combined column is shown if useCombinedRenderer is true. I wanted to do a ternary, but it seemed like I wouldn't be able to have logical checks (eg precision != 'hours') within a set of {} if it's not at the beginning?
• When precision='seconds' the reaction speed of the timepicker is noticeable slow. That's also why I have increased the time allowance in some of the tests to 50000 ms. This is because I'm iterating through the times a significantly higher amount - 24 hrs * 60 mins * 60 secs vs the previous 24 hrs + 60 mins + 60 secs. I'm not sure how to alleviate this issue, but I think the relevant code is found in lines 32-54 and 333-391.

Notes:

• I added the ...Renderer properties to Datepicker so that users can customize the attached Timepicker.
• I removed the hourRenderer, minuteRenderer, secondRenderer, and meridiemRenderer from the stories for Timepicker because they were not visible in the control panel anyway and just appeared as dashes.
• I edited the LabeledInputs for the Timepicker stories so that they would show the relevant time precision.
• Is there a way to make the precision property in the control panels of the Timepicker stories show up as radio multichoice rather than typing it in?

Closes #507

Checklist

• Add meaningful unit tests for your component (verify that all lines are covered)
• Verify that all existing tests pass
• Add component features demo in Storybook (different stories)
• Approve test images for new stories
• Add screenshots of the key elements of the component

[netlify]

❌ Deploy Preview for itwinui failed.

Name	Link
🔨 Latest commit	d8788a1
🔍 Latest deploy log	https://app.netlify.com/sites/itwinui/deploys/633f045faab32d0008887be1

[elephantcatdog]

Updated visual tests

[elephantcatdog]

Keyboard nav is missing esc key functionality but I'm not sure where we could add it as "opened" state is controlled by the user.

It could be done like in Dialog: have a property 'onClose' (and maybe the property 'closeOnEsc' also) that takes a function that closes the component.

I think this can be a new issue though.

Made the issue: iTwin/iTwinUI#855

[gretanausedaite]

Don't forget to add issues that was discussed in this PR (eg. test improvements, etc)

[elephantcatdog]

Don't forget to add issues that was discussed in this PR (eg. test improvements, etc)

Added issue for test improvements: iTwin/iTwinUI#856

[elephantcatdog]

Another problem I found is that if the selected time is not 'visible' in the DOM, pressing tab from the menu will not allow you to enter the time dropdown.

[elephantcatdog]

I will revert virtualization changes and make an issue for it.

[r100-stack]

One more issue I noticed is that using PageUp/PageDown keys in the VirtualizedTimePickerColumn do not scroll beyond the point that the selected time is visible. In the below example, I first pressed the PageDown button a couple of times, and then the PageUp.

This can be addressed in the new PR that adds virtualization to TimePicker

Video1.webm

[elephantcatdog]

Reverted to before virtualization. Will make a new issue in the morning and consolidate the others I already made.

Outdated

[gretanausedaite]

Visual tests fail because focused time changed position from start to end.
To make visual tests more stable you can change scrollIntoView position to center, or start/end.

[elephantcatdog]

Visual tests fail because focused time changed position from start to end.
To make visual tests more stable you can change scrollIntoView position to center, or start/end.

When I do this, clicking on a new time moves it to the center/end/start. Is that okay? It seems worse than before. I tried to add behavior: 'smooth' because I thought that at least it would visually scroll, but I can't because this definition of scrollIntoViewOptions doesn't include behavior.

Core._.TimePicker.-.Combined.Storybook.-.Google.Chrome.2022-12-16.08-06-15.mp4

[gretanausedaite]

When I do this, clicking on a new time moves it to the center/end/start. Is that okay? It seems worse than before. I tried to add behavior: 'smooth' because I thought that at least it would visually scroll, but I can't because this definition of scrollIntoViewOptions doesn't include behavior.

No, leave it as is, just update images. Let's hope it changed because of virtualisation.

[r100-stack]

One small thing to change could be to increase the timeout on the combined renderer tests because they fail for me sometimes. But that may be something just on my end, not sure.

[elephantcatdog]

nit: I think this got readded

This actually does change the look; I just didn't realize it before.

With div:

Without:

[elephantcatdog]

I hijacked the previous bug I filed and changed it to encompass all of virtualized timepicker: iTwin/iTwinUI#856