titanium-sdk
titanium-sdk copied to clipboard
[Breaking change] Android: Ti.UI.Color parity
- [x]
Ti.UI.fetchSemanticColor
returnsTi.UI.Color
- [x] Support
Ti.UI.Color
instances for components properties - [x] Automatically change color value on dark/light mode switch
This PR includes commit from #13267 and #13265 Fix #13272
Read this before merging.
Fails | |
---|---|
:no_entry_sign: | Test reports missing for Android Main. This indicates that a build failed or the test app crashed |
Warnings | |
---|---|
:warning: | There is no linked JIRA ticket in the PR body. Please include the URL of the relevant JIRA ticket. If you need to, you may file a ticket on JIRA |
Messages | |
---|---|
:book: | :fist: The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready. |
:book: |
:white_check_mark: All tests are passing Nice one! All 14752 tests are passing. (There are 934 skipped tests not included in that total) |
:book: | :tada: Another contribution from our awesome community member, drauggres! Thanks again for helping us make Titanium SDK better. :thumbsup: |
Generated by :no_entry_sign: dangerJS against 7fc7d01c4ac0c2362dd70e9e7f72a32b0db19984
FYI: The reason we didn't add Ti.UI.Color
proxy support to Android is because it would be a HUGE breaking-change to existing modules since many of them blindly cast a color property's value to (String)
, which would cause a casting exception.
For example, your changes would break the following... https://github.com/appcelerator-modules/ti.map/blob/master/android/src/ti/map/CircleProxy.java#L163 https://github.com/appcelerator-modules/ti.map/blob/master/android/src/ti/map/PolygonProxy.java#L122 https://github.com/appcelerator-modules/ti.admob/blob/master/android/src/ti/admob/AdmobView.java#L116 https://github.com/appcelerator-modules/titanium-web-dialog/blob/master/android/src/ti/webdialog/Utils.java#L68
The backward compatible solution that we implemented embeds both dark/light color values into a single string when calling fetchSemanticColor()
. That's the "ti.semantic.color:dark=value;light=value"
string format I'm sure you've seen. That's the color string you want to assign to a view proxy's properties so that it will support dark/light theme switching... and this definitely works.
Anyways, that's my 2 cents. This March, you guys can handle this anyway you want. If you want to move forward with this, then my recommendation is to do this in a new Titanium 11.0.0 release and force everyone to rebuild their modules.
Thanks @jquick-axway, I'm glad you are still with us.
Very impressive PR! So thrilled to get this reviewed and merged when TiDev can take over.
@drauggres A few merge conflicts to resolve here, but no time pressure as it's for 11.0.0 (targeted for September) anyways.
A few merge conflicts to resolve here, but no time pressure as it's for 11.0.0 (targeted for September) anyways.
This happened bacause this PR had to include commits from #13267 and #13265 and they were "merged" with rebase, i.e. not really merged (in git terms) but copies of the commits were created.
I rebased my branch, should be fine now.
@m1ga And this one as well