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 android nougat display spinner #118

Merged
merged 2 commits into from
Apr 9, 2020
Merged

Conversation

luancurti
Copy link
Member

@luancurti luancurti commented Feb 23, 2020

Summary

Display spinner should respect display prop

Closes #117

Test Plan

Run this demo https://github.com/otaviogaiao/teste-datepicker and date picker with mode spinner and date picker with spinner should be displayed

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
Android ✅❌

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

@vonovak
Copy link
Member

vonovak commented Mar 2, 2020

Let me please take a look at this before we merge, thanks!

@luancurti
Copy link
Member Author

Let me please take a look at this before we merge, thanks!

Of Course!

@lochness42
Copy link
Collaborator

That's why I didn't merge it yet

@TreeMan360
Copy link

Any idea when this will be merged in?

@TreeMan360
Copy link

I thought this was going to make it into SDK 37 ;(

@vonovak
Copy link
Member

vonovak commented Apr 8, 2020

@luancurti are you planning to look into this? Thanks :)

@luancurti
Copy link
Member Author

@luancurti are you planning to look into this? Thanks :)

Yes, I'll make the suggested changes before the weekend

@luancurti luancurti force-pushed the fix/android-display-datepicker branch from 385c9eb to f3947e9 Compare April 9, 2020 02:40
@luancurti luancurti requested a review from vonovak April 9, 2020 02:59
@luancurti
Copy link
Member Author

Tested on Android Nougat 7.0 SDK 24

datetimepicker

@vonovak vonovak merged commit 0394eb8 into master Apr 9, 2020
@TreeMan360
Copy link

TreeMan360 commented Apr 11, 2020

This does not fix the problem for me once deployed as a standalone app (expo). I still get the 'default' clock when I select 'spinner'.

This happens with ALL our Android test devices.

@vonovak
Copy link
Member

vonovak commented Apr 11, 2020

@TreeMan360 as you pointed out, this fix is not available in expo 37 (managed workflow), so I'm unsure how you're reproducing it. Please open a new issue and provide a runnable reproduction (follow the issue template), or alternatively, consider contributing a PR with a fix for your issue, thank you.

@TreeMan360
Copy link

TreeMan360 commented Apr 11, 2020

@TreeMan360 as you pointed out, this fix is not available in expo 37 (managed workflow), so I'm unsure how you're reproducing it. Please open a new issue and provide a runnable reproduction (follow the issue template), or alternatively, consider contributing a PR with a fix for your issue, thank you.

I just referenced the new package and ignored the expo warning saying it may not be compatible. I assume from your response that this is not plausible (I would like to understand why?) so I guess I will either be forced to not use this package or wait until 38 (far from ideal).

@vonovak
Copy link
Member

vonovak commented Apr 11, 2020

Expo sdk ships native code from one of the previous versions of this package (use expo install to find which one), so even if you manually install the latest version of this package you will not actually run the latest version.

But note that the bug which was fixed on this PR only exists in Android version 7, and it's not a bug that would prevent user from picking a date.

@TreeMan360
Copy link

Ok thanks for the explanation that makes sense. My problem is simply that once I make a standalone app the “spinner” display is not used. This is across multiple android versions, I always get the clock. It’s fine in development. So is this a separate issue?

@vonovak
Copy link
Member

vonovak commented Apr 11, 2020

This sounds like a separate issue. But if it's fine in development but wrong in release build, that's pretty weird. You can try opening a new issue but please note we require runnable reproduction to consider the issue as valid - and that might not be easy in this case.
It might be best if you investigate this yourself because I think it'll take a while before maintenainers will have time to look into this.

@luancurti luancurti deleted the fix/android-display-datepicker branch April 29, 2020 01:52
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.

DatePicker on spinner mode not working on some Phones
4 participants