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

feat(ios): support new date picker styles #11977

Merged
merged 1 commit into from Sep 2, 2020

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn commented Aug 28, 2020

JIRA: https://jira.appcelerator.org/browse/TIMOB-28104

Note: Without these, default pickers will look off on iOS 14+, so developers should very carefully check their apps for pickers.

@build
Copy link
Contributor

build commented Aug 28, 2020

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

🚫 Tests have failed, see below for more information.
Messages
📖 🎉 Another contribution from our awesome community member, hansemannn! Thanks again for helping us make Titanium SDK better. 👍
📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 2 tests have failed There are 2 tests failing and 472 skipped out of 5539 total tests.

Tests:

ClassnameNameTimeError
ios.iphone.Titanium.Network.Socket.TCP#connect() and #write() async (14.0)7.821
Error: done() invoked with non-Error: [object Object]
file:///Users/build/Library/Developer/CoreSimulator/Devices/B98CCEEE-6478-4929-BE6A-C6DB04CAF126/data/Containers/Bundle/Application/9EFDA7D5-DB4E-4E17-8FC1-8E826A21ADD9/mocha.app/ti-mocha.js:4361:52
error@file:///Users/build/Library/Developer/CoreSimulator/Devices/B98CCEEE-6478-4929-BE6A-C6DB04CAF126/data/Containers/Bundle/Application/9EFDA7D5-DB4E-4E17-8FC1-8E826A21ADD9/mocha.app/ti.network.socket.tcp.test.js:133:15
ios.iphone.Titanium.Network.Socket.TCP#connect() (14.0)7.866
Error: done() invoked with non-Error: [object Object]
file:///Users/build/Library/Developer/CoreSimulator/Devices/B98CCEEE-6478-4929-BE6A-C6DB04CAF126/data/Containers/Bundle/Application/9EFDA7D5-DB4E-4E17-8FC1-8E826A21ADD9/mocha.app/ti-mocha.js:4361:52
error@file:///Users/build/Library/Developer/CoreSimulator/Devices/B98CCEEE-6478-4929-BE6A-C6DB04CAF126/data/Containers/Bundle/Application/9EFDA7D5-DB4E-4E17-8FC1-8E826A21ADD9/mocha.app/ti.network.socket.tcp.test.js:30:15

Generated by 🚫 dangerJS against 9049580

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code changes LGTM, we should add some unit tests that verify the new constants exist in JS, maybe a UI test that validates once set we get the sort of UI we expect?

@ssekhri
Copy link

ssekhri commented Sep 1, 2020

The datePickerStyle of DATE_PICKER_STYLE_COMPACT is mentioned as min iOS 13.4. Should it not be min iOS 14.0?

@hansemannn
Copy link
Collaborator Author

@ssekhri
Copy link

ssekhri commented Sep 1, 2020

@hansemannn thanks for the reference. However with the current PR the DATE_PICKER_STYLE_COMPACT does not show up as compact style on iOS 13.4+ device/simulator. It works fine on iOS 14. On iOS 13.4+ it still shows as wheel style.

@hansemannn
Copy link
Collaborator Author

It is primarily used for iOS 13.4 apps that use Catalyst (macOS support), thats why I added it here already. In iOS 14, it becomes available on the "normal" / non-Catalyst iOS apps as well. Should I change it?

@vijaysingh-axway
Copy link
Contributor

It is primarily used for iOS 13.4 apps that use Catalyst (macOS support), thats why I added it here already. In iOS 14, it becomes available on the "normal" / non-Catalyst iOS apps as well. Should I change it?

I don't think we should change it. As it is marked as iOS 13.4+ in apple doc, we should also have the same. Not working properly on iOS 13.4, might be apple bug. @ssekhri May be you can try on iOS 13.5, 13.6 and 13.7.

@hansemannn
Copy link
Collaborator Author

I mean, it will work on iOS 13.4 combined with Catalyst. So I think it should be fine, especially because #11955 is rolling in soon.

@ssekhri
Copy link

ssekhri commented Sep 1, 2020

I agree that we should be in sync with Apple docs on the version support. However it would be good to have it mentioned in the docs that DATE_PICKER_STYLE_COMPACT style would not show as expected on iOS <14 until used with Mac Catalyst.

@hansemannn
Copy link
Collaborator Author

@ssekhri I agree - and added the notes.

@ssekhri
Copy link

ssekhri commented Sep 1, 2020

FR Passed.
The datePickerStyle property working fine on iOS 13.4+ and iOS 14.
The property can be successfully set to DATE_PICKER_STYLE_AUTOMATIC, DATE_PICKER_STYLE_WHEELS, DATE_PICKER_STYLE_COMPACT, DATE_PICKER_STYLE_INLINE.

Verified on:
Mac OS: 10.15.4
SDK: 9.2.0.v20200831152744
Appc CLI: 8.1.0
JDK: 11.0.4
Node: 10.17.0
Studio: 6.0.0.202005141803
Xcode: 12.0 Beta6
iPhone 7Plus(v14.0 Beta6), iOS simulator 13.5

Note: Without these, default pickers will look off on iOS 14+, so
developers should very carefully check their apps for pickers.

Fixes TIMOB-28104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants