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

Refactor time picker #151

Merged
merged 14 commits into from
May 19, 2023
Merged

Refactor time picker #151

merged 14 commits into from
May 19, 2023

Conversation

qroll
Copy link
Contributor

@qroll qroll commented May 17, 2023

Changes

  • [delete] branch

Refactoring/updating time picker inputs

  • extract shared timepicker dropdown component
  • moved timepicker helper to utils folder and renamed to TimeHelper
  • align styles with Figma and DateInput
  • added readonly state to the original Timepicker

Other changes

  • the indicator bar is now red for error state
  • a plain text input is used in various components, have extracted it and placed it together with InputWrapper
  • updated DateInput and TimeRangePicker to use InputWrapper

Additional information

@qroll qroll requested a review from keithtxw May 17, 2023 06:03
@@ -399,6 +394,7 @@ export const TimepickerDropdown = ({
<Container
ref={resizeDetector.ref}
data-testid={getTestId("timepicker-dropdown")}
inert={show ? 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.

when set, the inert attribute disables interactivity with the element and its children. this includes exclusion from the tab sequence

it's supported in the majority of browsers now but unfortunately still not included in React, so a workaround is to augment the types and whitelist it from eslint

@keithtxw
Copy link
Contributor

Can we also change the TimeRangeInputValue to just start and end. And to add jsdocs for format, value and onChange

@qroll qroll force-pushed the refactor-time-picker branch from d931d29 to dc1db1c Compare May 18, 2023 08:17
@qroll qroll force-pushed the refactor-time-picker branch from dc1db1c to ffd81f1 Compare May 18, 2023 08:53
document.removeEventListener("mousedown", handleMouseDownEvent);
document.removeEventListener("keyup", handleKeyUpEvent);
};
}, [showEndTimeSelector]);
Copy link
Contributor Author

@qroll qroll May 18, 2023

Choose a reason for hiding this comment

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

I missed out on this during review. these callbacks have stale state. have also amended similar issues in Timepicker

@qroll qroll requested a review from keithtxw May 18, 2023 11:13
@keithtxw keithtxw merged commit ae6d4af into v2 May 19, 2023
@keithtxw keithtxw deleted the refactor-time-picker branch May 19, 2023 04:14
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