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

CARDS-1761: Adapt React Date / Time pickers #1147

Open
wants to merge 29 commits into
base: dev
Choose a base branch
from
Open

Conversation

veronikaslc
Copy link
Contributor

@veronikaslc veronikaslc commented Aug 3, 2022

CARDS-1761
CARDS-1926
Affected components:

  • Filters
  • Date/Time questions
  • Downtime Warning Configuration
  • Patient identification screen

@sashaandjic

This comment was marked as resolved.

@sashaandjic sashaandjic added the bug Something isn't working label Aug 12, 2022
@veronikaslc
Copy link
Contributor Author

veronikaslc commented Aug 15, 2022

Last screenshot has is observed in dev, notsure it is an issue though

@marta- marta- added Test me! Ready for testing and removed bug Something isn't working labels Aug 15, 2022
@marta-
Copy link
Contributor

marta- commented Aug 15, 2022

@veronikaslc I think the new picker is friendly enough to use on the patient identification screen. Please replace the DropdownsDatePicker with it. Do not remove the DropdownsDatePicker component from the code base, even if it's no longer used.

Copy link
Contributor

@marta- marta- left a comment

Choose a reason for hiding this comment

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

A couple of imperfections to be tackled:

  • [UI issue] The vertical alignment for date intervals (rendered as input dash input) was already not looking great in dev, but now with the date control having become taller due to the format being displayed in the label above the input, the dash looks badly misplaced. See the Date Formats Test questionnaire in the --test mode for example. Suggested fix: display: flex & align-items: baseline on the container of the input dash input + a margin-left&right: 8px on the dash.
  • [UX issue] Speaking of the format being displayed in the label above the input, I don't think we really need to enforce InputLabelProps={{shrink: true}}. I would let the label overlap the input when the value is empty.
  • [Bug] Time questions appear to be initialized with the current time. To replicate: add a question with the dataType: time to the Date Formats Test questionnaire, then create a form and note how the time input is rendered for a new, empty form.
    • Also, time questions with all possible formats (mm:ss, hh:mm, hh:mm:ss) should be added by default to the Date Formats Test.

Things not introduced by this PR, but nice to have since we're altering this code anyway:

  • The date question's error message should be styled like in the NumberQuestion.
  • In view mode, the date intervals should be displayed as from - to, like it is done for NumberQuestion, instead of from \n to

@veronikaslc
Copy link
Contributor Author

Rebased on the latest dev and addressed comments.

@veronikaslc veronikaslc added the Test me! Ready for testing label Aug 16, 2022
Copy link
Contributor

@marta- marta- left a comment

Choose a reason for hiding this comment

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

  • Patient login page: the date of birth input should be as wide as the entire row. The expected format should be displayed to the user; i suggest displaying it as helper text under the input
  • There should be a way to CLEAR the value of a date or time input.

@marta-

This comment was marked as resolved.

Copy link
Member

@sdumitriu sdumitriu left a comment

Choose a reason for hiding this comment

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

Bug: in proms mode, on the patient login page, selecting a DoB with the picker ends up displaying the day before, even though the actual value in the component and which is sent to the server is the correct one.

Visual bug (that Marta kind of complained about): since the input is too narrow, the value doesn't fit properly, so the date displayed is 2022-08-1 , with the last digit missing. Worse when a time value is also there next to the date.

@sdumitriu sdumitriu added the bug Something isn't working label Aug 17, 2022
@marta- marta- removed the Test me! Ready for testing label Aug 17, 2022
@veronikaslc veronikaslc force-pushed the CARDS-1761 branch 5 times, most recently from 38424de to 72bf5ad Compare August 18, 2022 01:47
veronikaslc and others added 27 commits August 31, 2023 20:51
…d by, Last modification date in the dashboard and on the Forms page
Proof of concept for the TimePicker component
Adapted DateTimePicker to the Question components
Refactored DateQuestionMonth and DateQuestionFull components into one;
Adapted DatePicker in the DowntimeWarningConfiguration component;
Adapted DateTimePicker in the DateFilter component
fixed DowntimeWarningConfiguretion file
Addressed codereview comments
Addressed codereview comments
Addressed codereview comments
Bug fix: on the patient login page, selecting a DoB with the picker ends up displaying the day before, even though the actual value in the component and which is sent to the server is the correct one.
Patient identification dob: more detailed placeholder, remove redundant helper text
Removed all unused props from date time pickers components
Fixed Number extra wide inputs
Use DatePickers when appropriate
Addressed codereview comments
Error message should appear under the invalid input only in the range
The error state of the input should be on only if the date in that input is invalid.
* Prevent js console errors on live date validation feedback
* Prevent invalid date message from stretching the date input to its length
Revert accidental chmod 644->755 introduced by commit 72bf5ad
- Added meridiem determination to the format
- More accurate picker view selection based on the format
Restricted time question format to the selection of choices
Adapt date/time utilities for padding and hour formatting variations
Fix the date hour format to 24hr with padding
Fixed timepicker for hh:ss format
Added type for date questions in the quiestionnaire wisard
Uniform error styling across range questions
Removed redundant import after rebase
Bump @mui/x-date-pickers to the latest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Test me! Ready for testing testing... Testing in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants