Skip to content

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.

3 participants