-
-
Notifications
You must be signed in to change notification settings - Fork 455
feat: date picker startOnYearSelection #1004
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: date picker startOnYearSelection #1004
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Generally this seem okay. I added some comments, please take a look.
Also, I'm not quite sold on the showYearPickerFirst prop name. I'm thinking maybe startOnYearSelection, or initialSelection 'year' | 'date'... What would you suggest?
Also, tests need to pass. Maybe just re-running them will help.
Thank you!
android/src/main/java/com/reactcommunity/rndatetimepicker/RNMaterialDatePicker.kt
Show resolved
Hide resolved
android/src/main/java/com/reactcommunity/rndatetimepicker/RNDatePickerDialogFragment.java
Outdated
Show resolved
Hide resolved
- Remove no force unwrap - Add a guard clause - Add observer cleanup react-native-datetimepicker#1004 (comment)
|
@vonovak I understand you’re not quite sold on the showYearPickerFirst prop name — could you clarify a bit more about what specifically doesn’t feel right with it? |
@Tatsunori-Morita It's a bit subjective, I suppose. Can you rename it to |
- Remove no force unwrap - Add a guard clause - Add observer cleanup react-native-datetimepicker#1004 (comment)
d23930a to
b85e32f
Compare
|
@vonovak However, some tests are still failing on CI, but I’m unable to reproduce the failures in my local environment. Thanks for your help! |
|
Thank you for the PR! The e2e tests were most likely failing due to content not fitting on screen. :) |
Summary
This pull request addresses issue #67 by adding a feature that allows the year picker to be shown first.
Implementation details:
Test Plan
Tested using an Android emulator.
pixcel6-android12-default_WStd3B14.mp4
pixcel6-android12-material_X3kctEEi.mp4
pixcel7-android14-default_D6ZmfYhW.mp4
pixcel7-android14-material_qUr9dRH2.mp4
nexus5x-android7-default_bIXOxaYC.mp4
nexus5x-android7-material_W1Lry5gS.mp4
What's required for testing (prerequisites)?
What are the steps to reproduce (after prerequisites)?
Compatibility
Checklist
README.mdexample/App.js)