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

Call Taker Time Picker Support External Updates #933

Merged
merged 6 commits into from
Jul 11, 2023

Conversation

miles-grant-ibigroup
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup commented Jun 28, 2023

Description:

There were some regressions that got introduced here. This PR resolves these with a second state for the text input, separate from the actual time input.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Please elaborate on what problem was supposed to be fixed. Also, this PR introduces a regression that prevents entering shorthand time notation such as "9p" (for 9 pm) or "1145" (military format), that regression should be addressed.

@miles-grant-ibigroup
Copy link
Collaborator Author

Good catch thanks! Let me know how f28dc50 works for you

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

The fix seems to work, however there remains one glitch when refreshing the page/pasting URL in the browser.

lib/components/form/call-taker/date-time-picker.tsx Outdated Show resolved Hide resolved
// Don't update if still typing
if (timeRef.current !== document.activeElement) {
setTypedTime(
safeFormat(dateTime, timeFormat, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to happen on initial render of the component (for situations such as pasting the URL into the browser), otherwise the "typed" time can be different from the status time as shown in the screenshot below.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch thanks! Let me add an initial load update

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Try 3ccd8d2

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Works much better, thanks for the changes!

lib/components/form/call-taker/date-time-picker.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@philip-cline philip-cline left a comment

Choose a reason for hiding this comment

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

LGTM, any idea what the deal is with the useIntl() hook being weird?

@miles-grant-ibigroup miles-grant-ibigroup deleted the call-taker-alllow-date-external-input branch July 11, 2023 19:34
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.

4 participants