Skip to content

fix(core-android): ActivityIndicator changed transferred its color to other ActivityIndicators of the same page #9026

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

Merged
merged 3 commits into from
Dec 28, 2020

Conversation

CatchABus
Copy link
Contributor

@CatchABus CatchABus commented Nov 12, 2020

PR Checklist

What is the current behavior?

Changing color to an ActivityIndicator applies the new color to all other ActivityIndicators in the same page.
That only applies to Android.

What is the new behavior?

Each indicator has independent color by calling mutate() on indicator drawable instance at the creation of view.

Fixes/Closes #9021 .

@cla-bot cla-bot bot added the cla: yes label Nov 12, 2020
@farfromrefug
Copy link
Collaborator

Wouldn t it be better to mutate only when setting the color? If all your activity indicators use the theme color no need to mutate anythin

@CatchABus
Copy link
Contributor Author

CatchABus commented Nov 12, 2020

Wouldn t it be better to mutate only when setting the color? If all your activity indicators use the theme color no need to mutate anythin

That is because an indicator's color can be changed multiple times depending on one's needs, resulting in pointless mutate() calls even though they have no appliance after first call. Initializing it once seemed good to me, but we can change it.

@farfromrefug
Copy link
Collaborator

Then mark the indicator as mutated. Oersonnaly as I use theme color for indicators I see that PR as a perf lost. I understand the need but we should do it right and mutate only when needed

@cla-bot
Copy link

cla-bot bot commented Nov 12, 2020

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Dimitris - Rafail Katsampas.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla: yes label Nov 12, 2020
@CatchABus
Copy link
Contributor Author

CatchABus commented Nov 12, 2020

Then mark the indicator as mutated. Oersonnaly as I use theme color for indicators I see that PR as a perf lost. I understand the need but we should do it right and mutate only when needed

You are right! To be honest, I didn't consider theme-based indicators. I just committed few more changes that might make things better.

@cla-bot cla-bot bot added the cla: yes label Nov 12, 2020
@rigor789 rigor789 added this to the 7.1 milestone Dec 20, 2020
@rigor789 rigor789 merged commit e16bc60 into NativeScript:master Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(core-android): Setting ActivityIndicator color changes the color of other ActivityIndicators
3 participants