Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Only change out colors if they have actually changed #10158

Merged
merged 1 commit into from Apr 2, 2020
Merged

Conversation

PureWeen
Copy link
Contributor

@PureWeen PureWeen commented Apr 1, 2020

Description of Change

  • Don't swap out background drawable if nothing has changed
  • Don't change any toolbar drawables or colors if nothings has changed

Issues Resolved

Platforms Affected

  • Android

Testing Procedure

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@PureWeen PureWeen added a/shell 🐚 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. labels Apr 1, 2020
@PureWeen PureWeen added this to To do in v4.6.0 via automation Apr 1, 2020
@PureWeen PureWeen moved this from To do to In Review in v4.6.0 Apr 1, 2020
using (var colorDrawable = new ColorDrawable(background.ToAndroid(ShellRenderer.DefaultBackgroundColor)))
toolbar.SetBackground(colorDrawable);
toolbarTracker.TintColor = foreground.IsDefault ? ShellRenderer.DefaultForegroundColor : foreground;
if (_titleTextColor != titleArgb)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting these values was causing a slight flickering of the hamburger icon when you popped a page

@PureWeen PureWeen merged commit b0e2cb2 into 4.6.0 Apr 2, 2020
v4.6.0 automation moved this from In Review to Done Apr 2, 2020
@PureWeen PureWeen deleted the fix_8581 branch April 2, 2020 16:09
@samhouts samhouts added this to the 4.6.0 milestone Apr 9, 2020
@samhouts samhouts added this to Done in Shell Apr 10, 2020
@samhouts samhouts added this to Done in Sprint 168 Apr 30, 2020
Copy link

@ahmedalejo ahmedalejo left a comment

Choose a reason for hiding this comment

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

Code readability and maintainability suggestion.

decorView.SetBackground(split);
if (!(decorView.Background is SplitDrawable splitDrawable) ||
splitDrawable.Color != color || splitDrawable.TopSize != statusBarHeight || splitDrawable.BottomSize != navigationBarHeight)
{
Copy link

@ahmedalejo ahmedalejo May 3, 2020

Choose a reason for hiding this comment

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

Large conditions should be moved to a method to express the intent of what it´s trying to detect

@samhouts samhouts removed this from Done in Shell May 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/shell 🐚 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. p/Android t/bug 🐛
Projects
No open projects
Sprint 168
  
Done
v4.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants