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

changed range in datepicker to 100 years ago to current year #25

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

Conversation

amans330
Copy link

No description provided.

@amans330
Copy link
Author

Hi @robotdan please review.

@robotdan
Copy link
Member

Can you describe the fix, what was broken and how did this fix it? Also, missing tests.

@amans330
Copy link
Author

The fix: The start year(first year in the list) was 10 years before the current year. I changed it to 100 years before the current year. The end year(last year in the list) was 10 years after the current year. I changed it to be the current year.
Earlier, we couldn't select the birth year, if the user is born before 2010. No point in displaying future years as well for this use case (However, it may be required for a case like "valid till" field, which uses this datepicker widget and would need to display future years as well).

…nd make sure it contains each year after that. Total 100 entries should be present.
@amans330
Copy link
Author

Test case added.

@robotdan
Copy link
Member

From a library perspective, this is just a date picker, the usage of a birthday selector is a specific use case we have in FusionAuth.

So, the ability to select a future year, while it doesn't make sense for a birthday picker, it may for other usages. I would hesitate to hard code assuming it is going to be used for a birthday picker.

So there are a few options,

  1. Change the default (this is what you have here)
  2. Change the default and provide configuration to allow it to be customized
  3. Leave the existing values, and provide configuration to allow it to be customized

Thoughts?

@amans330
Copy link
Author

Yeah, hardcoding it to show till the current year doesn't seem right to me either (last line in my previous comment). But providing configuration for start and end years sounds like overkill to me here. We can give the range from the last 100 to the next 100 years and that would solve most of the cases. Let me know what would you like to be done.

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