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

Enhanced date entry #392

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

Conversation

aDramaQueen
Copy link

Hi,
I added more convenience functionality to the DateEntry Widget:

  • You can now easily enable/disable this Widget by simply calling the dedicated methods
  • You can now change the date programmatically
  • You can now provide/change a window title for the PopUp window of the calendar
  • You can now provide a callback function, that is triggered automatically if the date has changed
  • Added some type hints

NOTE:
As I understood, you are translating some of the predefined texts in this library? You may add the pre defined window title to the pool of these pre translated texts.


TODO:
You may considering adding some tests for the formater string. Right now you assume the given string represents a valid date formate. What happens if I give something like: %H:%M:%S

This is a perfectly valid formating string, but is absolutely useless for a date picker.

Added more convenience functionality to DateEntry widget
New sanitizer does remove everything from given datetime object, except year, month and day.
@Aareon
Copy link

Aareon commented Nov 18, 2023

Please add tests, otherwise LGTM

@aDramaQueen
Copy link
Author

aDramaQueen commented Aug 24, 2024

Hi, what tests? The one I described in the TODO section or tests for the widget itself?

@Aareon
Copy link

Aareon commented Sep 3, 2024

I believe just formatter string tests would be sufficient. I'm hoping this gets merged soon :)

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