Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPHJOD-861: Update ark UI to version 4.12.0 and implement required changes #171

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

juhaniko
Copy link
Contributor

Description

  • Updated ARK UI to the latest version (4.12.0)
  • Datepicker: Remove redundant onInput listener in text input that messed up the value
  • Datepicker: Now uses parseDate from ARK UI to set the internal value in correct format as it was changed
  • Datepicker: In latest version the date/number buttons became div elements instead of button. Used asChild to make them buttons again so that they can be disabled
  • Slider: Unit test update
  • ConfirmationDialog: Unit tests needed the actions to be wrapped in act

Related JIRA ticket

https://jira.eduuni.fi/browse/OPHJOD-861

@@ -83,13 +98,6 @@ export const Datepicker = React.forwardRef<HTMLInputElement, DatepickerProps>(fu
name={name}
placeholder={placeholder}
className="ds-w-full ds-rounded-l ds-border-y ds-border-l ds-border-border-gray ds-bg-white ds-p-5 ds-font-arial ds-text-black ds-outline-none placeholder:ds-text-inactive-gray"
onInput={(e: React.ChangeEvent<HTMLInputElement>) => {
Copy link
Contributor Author

@juhaniko juhaniko Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the redundant onInput handler. It didn't seem to do much except cause trouble.

Copy link

import { Slider } from './Slider';

const { ResizeObserver } = window;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResizeObserver is globally stubbed in vitest.setup.ts so no need to mock it here.


await press('right');
expect(onValueChangeMock).toHaveBeenCalledWith(25);
await press('right'); // For some reason, the "aria-valuenow" attribute is not updated without extra right press
Copy link
Contributor Author

@juhaniko juhaniko Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part drove me nuts. Even though the onValueChange is called with the right value, the aria-valuenow attribute would not update without double press here, no matter what I tried. When testing with storybook, the keyboard controls work fine, so I think the issue is within the test environment. Just in case, I tested all the available values.

Things I tried that failed:

return (
<ArkDatePicker.Root
onValueChange={(details) => {
onChange({
target: { name, value: details.valueAsString[0] ?? '' },
} as React.ChangeEvent<HTMLInputElement>);
}}
value={value ? [value] : []}
value={value ? [parseDate(value)] : undefined}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Value cannot be a string like it used to, so parseDate from ARK UI is used.

@sauanto sauanto self-requested a review October 22, 2024 06:14
@juhaniko juhaniko merged commit 29b29a9 into main Oct 22, 2024
4 checks passed
@juhaniko juhaniko deleted the OPHJOD-861 branch October 22, 2024 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants