Skip to content

[Android] Fix starting menu icon color #93

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.

Sign up for GitHub

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

Closed
FeralAI wants to merge 1 commit into from
Closed

[Android] Fix starting menu icon color #93

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion android/app/src/main/java/com/reactnativenavigation/views/RnnToolBar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class RnnToolBar extends Toolbar {
private DrawerLayout mDrawerLayout;
private ActionBarDrawerToggle mDrawerToggle;
private ArrayList<View> mMenuItems;
private int mButtonColor;
Copy link
Copy Markdown
Collaborator

@guyca guyca Jun 26, 2016

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

@FeralAI FeralAI Jun 26, 2016

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

@guyca guyca Jun 26, 2016

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

@FeralAI FeralAI Jun 26, 2016

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.


public RnnToolBar(Context context) {
super(context);
Expand All @@ -82,6 +83,8 @@ public void setScreens(List<Screen> screens) {
}

public void setStyle(Screen screen) {
mButtonColor = (screen.navBarButtonColor != null) ? screen.navBarButtonColor : Color.BLACK;

if (screen.toolBarColor != null) {
setBackgroundColor(screen.toolBarColor);
} else {
Expand Down Expand Up @@ -244,7 +247,7 @@ public void setNavUpButton(Screen screen) {
if (screen != null && screen.navBarButtonColor != null) {
navArrow.setColor(screen.navBarButtonColor);
} else {
navArrow.setColor(Color.BLACK);
navArrow.setColor(mButtonColor);
}
navIcon = navArrow;
}
Expand Down