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

feat: [DHIS2-15463] Use dhis2 ui calendarInput component in working list #3712

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

alaa-yahia
Copy link
Member

@alaa-yahia alaa-yahia commented Jul 11, 2024

Implements DHIS2-15463

@alaa-yahia alaa-yahia requested a review from a team as a code owner July 11, 2024 23:12
Copy link
Contributor

@simonadomnisoru simonadomnisoru left a comment

Choose a reason for hiding this comment

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

Hi @alaa-yahia,
I think there are 3 calendar-related Cypress tests in the working list that fail in EventWorkingListsUser.feature. Can you take a look at them?
Support for editable input and dd-mm-yyyy format is needed, therefore this PR is blocked until dhis2/ui#1504 is merged.

@alaa-yahia alaa-yahia force-pushed the DHIS2-15463-use-dhis2-ui-calendarinput-component-in-working-list branch from 6b331e2 to ba85e8d Compare August 7, 2024 07:51
@alaa-yahia
Copy link
Member Author

Hi @alaa-yahia, I think there are 3 calendar-related Cypress tests in the working list that fail in EventWorkingListsUser.feature. Can you take a look at them? Support for editable input and dd-mm-yyyy format is needed, therefore this PR is blocked until dhis2/ui#1504 is merged.

Hey @simonadomnisoru
I have updated the tests.
An issue persists where the calendar popup remains open when switching focus between input fields, only closing when clicking elsewhere.
I will investigate it further and update the tests if I was able to solve it.

@alaa-yahia alaa-yahia force-pushed the DHIS2-15463-use-dhis2-ui-calendarinput-component-in-working-list branch from ba85e8d to 7744732 Compare August 7, 2024 13:00
@simonadomnisoru
Copy link
Contributor

simonadomnisoru commented Aug 7, 2024

Hey @simonadomnisoru I have updated the tests. An issue persists where the calendar popup remains open when switching focus between input fields, only closing when clicking elsewhere. I will investigate it further and update the tests if I was able to solve it.

Hi Alaa, I noticed a few things that I believe need some improvement:

  1. The calendar popup also stays open when you manually enter the date and then press Enter (perhaps it is the same issue you mentioned when switching focus between input fields).
  2. While manually entering a date, the error "Date string is invalid, received" is displayed. Is it possible to only display this error after the user has finished typing (onBlur), the same way it currently works in play?
Screenshot 2024-08-07 at 12 42 43
  1. When the calendar is in the dd-mm-yyyy format and you select a date, it is displayed in the input in the wrong format and the error "Please provide a valid date" is visible. When you manually type in a date in the dd-mm-yyyy format it works fine. In the video bellow you can see more details.
dd-mm-yyyy.format.mov

Let me know if you need any help. Thank you!

package.json Outdated
@@ -19,6 +19,7 @@
"@dhis2/d2-ui-rich-text": "^7.4.0",
"@dhis2/d2-ui-sharing-dialog": "^7.3.3",
"@dhis2/ui": "^9.10.1",
"@dhis2-ui/calendar": "9.9.0-alpha.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

Alphas releases are useful when developing, but IMHO it is best to wait for the stable @dhis2/ui release before merging this PR. Do you know the timeline/steps to get the stable release to include the alpha changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was given the task to update the alpha with latest master. But I am not sure about the timeline as there are still accessibility task for CalendarInput.

Copy link
Contributor

@simonadomnisoru simonadomnisoru left a comment

Choose a reason for hiding this comment

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

LGTM! 👏

…s2-ui-calendarinput-component-in-working-list
Copy link

Copy link

@geethaalwan geethaalwan left a comment

Choose a reason for hiding this comment

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

Tested successfully on 2.42,2.41.2,2.40.5,2.39.7 versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants