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

[ListItem] rightIconButton doesn't prevent ripple on mobile #3556

Closed
EloB opened this issue Mar 2, 2016 · 15 comments
Closed

[ListItem] rightIconButton doesn't prevent ripple on mobile #3556

EloB opened this issue Mar 2, 2016 · 15 comments
Labels
bug 🐛 Something doesn't work

Comments

@EloB
Copy link
Contributor

EloB commented Mar 2, 2016

It works in desktop without the ripple.

Here is an example:

  • Visit https://mtg.zone (site only works on chrome and safari because I use websql)
  • Make your screen small so you see the icon on the top right menu
  • Add a card to deck
  • Click on the top right icon
  • Tap (with a touch event) on the zoom button to trigger the ripple

Touch tap
ripple

Mouse click
no-ripple

@nathanmarks nathanmarks added the bug 🐛 Something doesn't work label Mar 2, 2016
@heetvachhani
Copy link
Contributor

Hi @EloB I tried to reproduce this in desktop using step you described but for me it's working perfectly fine. I'm using Chrome (Version: 48.0.2564.116)
I've also checked in Safari(Version: 9.0.3) working fine there too.

Here's the Mouse Click example:

Alt Text

@nathanmarks
Copy link
Member

@heetvachhani What happens when you use chrome dev tools to simulate a touch?

That reproduces it for me:

image

@heetvachhani
Copy link
Contributor

Yes that reproduced for me 😁

@tintin1343
Copy link
Contributor

@EloB : I'm trying to fix this and was trying to reproduce it. So I wanted to understand the structure of the list that you are using.

Can you shed some light on this. Check this page from our docs site and see if you find issue with the nested lists.

@tintin1343
Copy link
Contributor

@EloB : So, I tried reproducing the issue you are having on the docs page and I was able to do it on the nested list example which looks similar to your use case.

I noticed the nested list-item did not let the touch ripple bubble up to the list-item parent. However the parent list-item labelled "inbox" behaved in the same way as your use-case.

touch-issue

I added the prop disabled={true} to the parent list-item labelled "inbox" and that solved the problem. The ripple can be seen on touch only but not on click of the list-item since its disabled. I bet the problem with your use-case is similar.

touch-solved

Let me know, if this solves the problem.

@nathanmarks : Is this an bug in our docs page? I see it as more of a use-case thing.

@EloB
Copy link
Contributor Author

EloB commented Mar 2, 2016

Im getting the bug on the docs as well.

ripple

@EloB
Copy link
Contributor Author

EloB commented Mar 2, 2016

I need to be able to click the list and also the rightButtonElement. So disable it will only make the rightButtonElement to work right?

@EloB
Copy link
Contributor Author

EloB commented Mar 2, 2016

I think the way ListItem triggers the ripples are kind of broken. On a phone it triggers the ripples everytime you touch them. Very annoying when you scroll the page. Maybe this is another bug. Should I make another issue with that? Try this on a phone and scroll down the page: https://mtg.zone/#/sets

@tintin1343
Copy link
Contributor

@EloB : Yes. I did enable touch on devtools. Thank you for explaining your use-case.
Yes you are right. The list-item won't be clickable but the icon will be. I misunderstood from your screengrab. Now that I know what you are trying to achieve, let me try and see if it is fixable.

Also, i checked your website on phone. The list-item, is not clickable when I tried it through my phone. only the right-icon is clickable.

@EloB
Copy link
Contributor Author

EloB commented Mar 2, 2016

By clicking on the ListItem it should decrease the left avatar number by one.

@tintin1343
Copy link
Contributor

The clicking the ListItem it should decrease the left avatar number by one.

Doesn't work in mobile chrome browser. It closes the side nav when I click on the list-item. Anyway, Let me see what I can do.

@EloB
Copy link
Contributor Author

EloB commented Mar 3, 2016

Oh that's because its the last item if you have more than one card in the deck. Then you will see that is decreases.

@tintin1343
Copy link
Contributor

@EloB : Yeah I did see that. Thanks!

@EloB
Copy link
Contributor Author

EloB commented Mar 4, 2016

Thanks for solving this issue! 👍

@tintin1343
Copy link
Contributor

@EloB : No worries man. Happy to help! Best of luck with your project.

tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 4, 2016
Add flex property to menuItem

Resolves mui#3531
tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 6, 2016
mbradleyis pushed a commit to staticinstance/material-ui that referenced this issue Mar 7, 2016
und3fined pushed a commit to und3fined/material-ui that referenced this issue Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

No branches or pull requests

4 participants