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): properly set ImageView tintColor #11441

Merged
merged 5 commits into from Apr 28, 2020

Conversation

hansemannn
Copy link
Collaborator

@build build added this to the 9.0.0 milestone Jan 19, 2020
@build build requested a review from a team January 19, 2020 19:41
@build
Copy link
Contributor

build commented Jan 19, 2020

Messages
📖

💾 Here's the generated SDK zipfile.

📖 🎉 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.
📖

✅ All tests are passing
Nice one! All 6627 tests are passing.
(There are 708 skipped tests not included in that total)

Generated by 🚫 dangerJS against 7b44bbf

Copy link
Contributor

@ypbnv ypbnv left a 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!
Looks good to me!

@jquick-axway
Copy link
Contributor

jquick-axway commented Jan 22, 2020

I've confirmed that this PR's code change makes Android's tint behave exactly like iOS' for both masked images and completely opaque images (such as camera photos).


Side Note:
The only thing I don't like is iOS and Android's tinting behavior of opaque images (no alpha; such as photos). I would expect a "tint" to apply a color blend effect like how it works in Photoshop. But instead you see a solid color (the tint color) completely covering the image. But the only way to achieve this color blending effect is to grayscale the image first, then apply the tint color. I'm pretty sure this can be achieved via a single custom color matrix to the image view.
(I don't think this should stop this PR. This is how it works on iOS. I just don't like the behavior.)

@hansemannn
Copy link
Collaborator Author

hansemannn commented Jan 22, 2020

@jquick-axway I made titanium-image-filters exactly for that :). Or you can use masked image filters that are available in the SDK as well.

@hansemannn
Copy link
Collaborator Author

@garymathews The unit test failures seem unrelated, but maybe critical: Error: expected Object {} to be '#ff1f1f'

@ewanharris ewanharris modified the milestones: 9.0.0, 9.1.0 Feb 12, 2020
@ssjsamir ssjsamir self-requested a review April 28, 2020 13:37
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed: Able to see the tint applied to the following test case (https://jira.appcelerator.org/browse/TIMOB-27721) :

const window = Ti.UI.createWindow();
const icon = Ti.UI.createImageView({
tintColor: 'red',
width: 30,
image: 'example.png'
});
window.add(icon);
window.open();

Test Environment

MacOS Catalina: 10.15.5 Beta
Xcode: 11.4
Java Version: 1.8.0_131
Android NDK: 21.1.6273396-beta2
Node.js: 10.16.3
""NPM":"5.0.0-1","CLI":"8.0.0-master.10""
Pixel Xl 7.1.1 Sim

@sgtcoolguy sgtcoolguy merged commit c2633b0 into tidev:master Apr 28, 2020
sgtcoolguy pushed a commit that referenced this pull request Apr 28, 2020
@hansemannn hansemannn deleted the TIMOB-27721 branch April 28, 2020 18:03
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

8 participants