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

[UI] Animation in menu missing in AMOLED theme & not noticable in black theme #141

Closed
rugk opened this issue Apr 26, 2016 · 11 comments
Closed

Comments

@rugk
Copy link

rugk commented Apr 26, 2016

In the swipe menu there should be a "press"/tap animation when tapping on a menu button. (So the user gets some feedback that the [correct] button is actually pressed) This is best practise and I am quite sure you can find some Google Design docs for that somewhere... (but I don't care to look for them now)

@brandonio21
Copy link
Contributor

@dnldsht Is this implemented? I was trying to recreate @rugk's complaint but couldn't do so.

From what I can tell, when you tap a button in the sliding menu, the appropriate tap animation plays.

@rugk
Copy link
Author

rugk commented Aug 10, 2016

I use the AMOLED theme, but I can reproduce this issue also with all other themes. When tapping on a menu item in the sliding menu no "pressed" animation is shown.

@brandonio21
Copy link
Contributor

@rukg - What version of Android are you using? On the White theme, I am seeing a dark grey tap animation for the sliding menu. On the dark theme, I am seeing a dark grey tap animation for the sliding menu. However, I am seeing no tap animation for the AMOLED theme. I'm assuming because the animation is also black.

I suspect that you are not seeing the tap animation for the white theme due to your Android version? I am using Marshmallow.

@rugk
Copy link
Author

rugk commented Aug 10, 2016

CM13 (Android 6)

Okay, so I tested it again and yes I really see a tap animation with the white theme and I also see the one for the black theme, but this one is really hard to notice.
As for the AMOLED theme there is indeed none and I also think it is because that's black, but well... just show a dark grey there. 😃

@rugk rugk changed the title [UI] Animation in menu missing [UI] Animation in menu missing in AMOLED theme & not noticable in black theme Aug 10, 2016
@brandonio21
Copy link
Contributor

@rugk - Great. I'll see if I can take a crack at it later today :)

@brandonio21
Copy link
Contributor

Sorry for the delay.. just got my home dev environment setup.

After searching through the code (for the first time) for about 2 hours, I think I am starting to understand how everything comes together. The animations that are referred to in this issue are "ripples" provided by MaterialRippleLayout.

There doesn't seem to be any reference in the code to explicitly setting ripple colors based on the theme. However, in ripple.xml there seems to be some sort of mask. I'm not sure if this has anything to do with the ripple colors.

@dnldsht - Can you perhaps comment on how the ripple coloring is determined? I am having a hard time finding it myself.

@gilnd
Copy link
Member

gilnd commented Aug 11, 2016

hi guys, to make the ripple effect i'm using the MaterialRippleLayout lib.
@brandonio21 In some other case i just use to add those tag:android:clickable="true" and 'android:background="?attr/selectableItemBackground" on a view.
I'm looking to another lib or a method that alow you to change the ripple color by code.
If you have find any solution or just want to know something about the code, tell me!

Cheerse.

@brandonio21
Copy link
Contributor

Just an FYI that I have made progress on this. I should have a PR out within the hour

@brandonio21
Copy link
Contributor

@rugk @Mow3l -- I have changed all of the button-style LinearLayout views to use the ripple drawable that comes in the Android Support Libraries. I have also changed the ripple color to be a half-grey so that it shows up on all themes.

Hopefully this fixes all issues. PR and relevant details are viewable #273

@dnldsht
Copy link
Member

dnldsht commented Aug 12, 2016

@brandonio21 good job! I merged your pull request, i also removed the MaterialRippleLayout lib, I also added this effect int the items of the GridView using android:foreground="@drawable/ripple"

dnldsht added a commit that referenced this issue Aug 12, 2016
@rugk
Copy link
Author

rugk commented Aug 12, 2016

@dnldsht You should not use the word "fixed" (in past) as it as bad for commit messages (which should rather be in present) and also prevents auto-closing of this issue.

@brandonio21 You can also use this syntax in PRs.

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

No branches or pull requests

4 participants