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

BottomNavigationView - selector not working #283

Closed
MFlisar opened this issue May 24, 2017 · 20 comments
Closed

BottomNavigationView - selector not working #283

MFlisar opened this issue May 24, 2017 · 20 comments
Assignees
Labels

Comments

@MFlisar
Copy link
Contributor

MFlisar commented May 24, 2017

Using BottomNavigationView does currently not work with IconicsDrawables. The weird thing is that the default state color tinting works fine, but the selected state color is not working.

Any ideas why? I checked the code and saw, tinting should be supported. And I check older issues and see that this should work, but it does not...

@mikepenz
Copy link
Owner

@MFlisar so the default color tinting of that view does not work?

How do you define the drawable? I remember there are some problems in the framework (for normal drawables at least) if you tint using the xml, you can no longer tint using code. There are also problems if you tint with a single color, and afterwards with a colorStateList.

Have you tried to directly supply a ColorStateList for tinting?

In addition it could also be related to this:
#278

@mikepenz mikepenz self-assigned this May 24, 2017
@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

The default color tinting is working, the selected color tinting is not working. I use following simple selector (for text + icon):

<?xml version="1.0" encoding="utf-8"?>
<selector xmlns:android="http://schemas.android.com/apk/res/android">
	<item android:state_checked="true" android:color="@color/md_orange_500" />
	<item android:color="@color/md_white_1000"  />
</selector>

I do following:

* I use `IconicsDrawable` in white, no selector, simple color
* I tried wrapping it in a CompatDrawable with no effect
* I tried loading the selector in code and passing it als `ColorStateList`, does not have any effect either

The BottomNavigationView defines following selectors:

app:itemIconTint="@color/bottom_bar_selector"
app:itemTextColor="@color/bottom_bar_selector"

Effect

My icons are always white, even the selected ones whereas the lables are white (if not selected) or orange (if selected). As the BottomNavigationView does use tinting I would assume I should not use a selector as color for the IconicsDrawable, sadly this does not work

@mikepenz
Copy link
Owner

It might be really related to the other problem of not switching the color later.

have you also tryed the android:state_selected also?

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

No, I haven't because the selector is working with a default drawable... But I'll add it and try it

Edit: does not help..

@mikepenz
Copy link
Owner

mikepenz commented May 24, 2017

Can you please try the previous version of Android-Iconics? I am curious if it really was that PR we merged

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

I'll try that. Sadly, the missing invalidation (that I posted in the linked issue) I found in code is not the case when the tinting is changed...

I'll try it later though, need to adjust a few things in code because I'm using the menu inflator wrapper from my pull request

@mikepenz
Copy link
Owner

Oh ok sadly. :/ would have been an easy fix :D

ok :)

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

I tried invalidating the drawable manually in OnNavigationItemSelectedListener as well and it has no effect...

Here's the code from the NavigationView. Maybe it's related to it?

 public void setIcon(Drawable icon) {
    if (icon != null) {
        Drawable.ConstantState state = icon.getConstantState();
        icon = DrawableCompat.wrap(state == null ? icon : state.newDrawable()).mutate();
        DrawableCompat.setTintList(icon, mIconTint);
    }
    mIcon.setImageDrawable(icon);
}

getConstantState is not overwritten in IconicsDrawable, not sure if it's necessary though, I actually have never made any deeper experience with drawables yet...

@mikepenz
Copy link
Owner

@MFlisar hmmm you tried the previous Iconics version right?

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

Not yet. Which version should I try? I don't know where I see into which version this PR was merged

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

I now tried v2.8.1, it does not support ColorStateList as color yet. It's working back than, at least something to start with :-)

@mikepenz
Copy link
Owner

yes

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

Tried v2.8.3 as well, everything working there as well...

And then I tried adding the changes one by one, following is breaking the behaviour already:

cc2d478

Changing only setState from v2.8.3 to the implementation above, breaks the behaviour for me...

@mikepenz
Copy link
Owner

oh wait
can you please try to call super.setState(stateSet); in addition?

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

Just wanted to try this

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

Tried the head branch and add the super call and it's working again :-)

@Override
public boolean setState(@NonNull int[] stateSet) {
    boolean b = super.setState(stateSet);
    return b || (mIconColor != null && mIconColor.isStateful() || mColorFilter != null || mTintFilter != null);
}

I'm not sure what I should do with the super return value though...

@mikepenz
Copy link
Owner

well looks good that way. Awasome. With that working again that means I will have to do a update. Possibly it also directly fixes: #278

@MFlisar
Copy link
Contributor Author

MFlisar commented May 24, 2017

Probably. I pushed the change to my branch, so it's part of my pull request now

@mikepenz
Copy link
Owner

Merged your PR

@anandgaur22
Copy link

bottomNavigation.setItemIconTintList(null);
remove this line then show icon colour

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants