Skip to content

[Android] Fix starting menu icon color#93

Closed
FeralAI wants to merge 1 commit into
wix:masterfrom
the-mx-group:android_menuicon_fix
Closed

[Android] Fix starting menu icon color#93
FeralAI wants to merge 1 commit into
wix:masterfrom
the-mx-group:android_menuicon_fix

Conversation

@FeralAI
Copy link
Copy Markdown

@FeralAI FeralAI commented Jun 25, 2016

The navBarButtonColor was only being applied to screens after the first. The fix ensures the color is applied to the starting screen as well.

@FeralAI FeralAI changed the title Fix setting menu icon color Android fix starting menu icon color Jun 25, 2016
@FeralAI FeralAI changed the title Android fix starting menu icon color [Android] Fix starting menu icon color Jun 25, 2016
private DrawerLayout mDrawerLayout;
private ActionBarDrawerToggle mDrawerToggle;
private ArrayList<View> mMenuItems;
private int mButtonColor;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@jskuby Lets not keep this as a member since we use it only in a single function

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@guyca When calling setNavUpButton without a screen it's implied we're navigating back. Having no screen object also uses the default color when styling the icon, which may not have been the color of the previous screen's menu icon. This may just be more of an argument for a previous comment of mine mentioning splitting up setNavUpButton into separate style and state methods.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm.. the next developer who would step into the code won't know that passing null to setNavUpButton implies it's called after navigating back.
Maybe if the screen is null - don't style the up button. Instead, we can style it in setStyle which is called after StyleHelper.updateStyles is called.
wdyt?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I like it. That also means we don't need to pass screen into setNavUpButton, it just needs to be able to set the correct state.

@FeralAI
Copy link
Copy Markdown
Author

FeralAI commented Jun 30, 2016

Pretty sure this will be resolved with the updates from PR #97.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants