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

Fix DateInput #314

Merged
merged 6 commits into from
Sep 27, 2024
Merged

Fix DateInput #314

merged 6 commits into from
Sep 27, 2024

Conversation

AnnMarieW
Copy link
Collaborator

fixes #308
fixes #249
fixes #203
fixes #264

This PR is based on PR #265 by @albavilanova

@albavilanova Thanks so much for your PR. This new one is based on your work and includes a few sample apps for testing. I was reluctant to update your PR because I understand you are using your own version of dmc in production. If you would like to continue to work on this, let me know and I'll close this PR in favor of yours. Otherwise we can carry on with this one and we'll give you credit in the Changelog when it's merged.

This PR also enables the date parsing to work with the DateInput component even when the DateProvider is not being used.

@snehil - was there any reason not to merge the original PR 265?

Note - The three sample apps for testing are in the usage-265 folder and can be deleted after merging.

@AnnMarieW AnnMarieW requested a review from snehilvj September 14, 2024 12:10
@AnnMarieW AnnMarieW added the ready Ready for final review & merge label Sep 22, 2024
@AnnMarieW AnnMarieW requested review from alexcjohnson and removed request for snehilvj September 25, 2024 18:52
@alexcjohnson
Copy link
Collaborator

Two usage questions:

  1. Does it work to have two different locales on different components?
  2. Does it work to change the locale of an component via callback?

@AnnMarieW
Copy link
Collaborator Author

The issue was that the locale wasn't working when the component first rendered. I'll try your solution and see if it works - I'll also verify with the two usage items too. Thanks for the suggestions 🙂

@AnnMarieW
Copy link
Collaborator Author

@alexcjohnson This is now ready - and verified that additional use-cases work 🎉

@albavilanova
Copy link

Thanks @AnnMarieW for working on this, I just used my own version because of this issue, but I will be back to using this one once it is updated in PyPI.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Fantastic! 💃

@AnnMarieW AnnMarieW merged commit 103be93 into snehilvj:master Sep 27, 2024
@AnnMarieW AnnMarieW deleted the pr-265-amw branch September 27, 2024 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Ready for final review & merge
Projects
None yet
3 participants