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

Button with ripples stays focused after mouse is moved away #1124

Closed
tinrab opened this issue Aug 14, 2017 · 15 comments · Fixed by #1800
Closed

Button with ripples stays focused after mouse is moved away #1124

tinrab opened this issue Aug 14, 2017 · 15 comments · Fixed by #1800
Labels

Comments

@tinrab
Copy link
Contributor

tinrab commented Aug 14, 2017

Clicking and holding a button, then releasing or clicking away from it, keeps the button in focused state.

An example on https://material-components-web.appspot.com/button.html.
mdc-button

The behavior is different with "CSS Only" buttons. You have to click outside of button's bounds to remove the "background tint", which also doesn't seem correct. With Material Design Lite it works as expected.

@Garbee
Copy link
Contributor

Garbee commented Aug 14, 2017

The behavior is different with "CSS Only" buttons. You have to click outside of button's bounds to remove the "background tint", which also doesn't seem correct.

This is absolutely correct behavior for CSS only. As the browser native states are relied on which provide the user-expected states to know where things are for future keyboard navigation or visual focus.

With Material Design Lite it works as expected.

Because MDL used JS-based buttons only and implemented the bluring post ripple. Which, is fine for JS-level solutions for high fidelity to the MD spec (as is what makes this a bug with the button + ripple in MCW.) But, this doesn't mean the basic CSS-only solution is incorrect in what it does. Accessibility first with the basics, JS to enhance and align completely to MCW where CSS-only doesn't allow.

@touficbatache
Copy link
Contributor

That's also happening on Android. I opened an issue on June 9th but it's not yet resolved till now. #809

@acdvorak
Copy link
Contributor

It looks like ripple is only listening for a mouseup event on its target element. It should probably listen for that event on the document instead.

@theenoahmason
Copy link

This behavior is forcing us to stop using all ripples on our current MDC project. Even touching a ripple surface on scroll triggers the ripple, effectively making ripples as they are implemented now unusable. After a solid scroll down a page, every ripple element I happened to touch is now rippled, with no way whatsoever of removing ripples.

Here's the problem with NOT using ripples: if im not using js ripples anywhere, I dont want them showing anywhere... unfortunately sometimes default behavior for other components triggers js ripples on child elements (ie: accept/cancel buttons in js dialog component, and tabs in js tab bar scroller component), so there's that.

@touficbatache
Copy link
Contributor

Exactly @theenoahmason! This is so annoying and is not getting enough attention!

Even touching a ripple surface on scroll triggers the ripple, effectively making ripples as they are implemented now unusable. After a solid scroll down a page, every ripple element I happened to touch is now rippled, with no way whatsoever of removing ripples.

That's happening with me too: I even opened an issue #1080 but it's still in-tracker. I am making a project with MDC-Web and it's complete. I just need these annoying bugs fixed before the release!!

@touficbatache
Copy link
Contributor

@theenoahmason Scroll triggering the ripple could be fixed with this simple trick https://jsfiddle.net/z_acharki/jnwrc5ay/132/

@mpiroc
Copy link

mpiroc commented Oct 11, 2017

@acdvorak FYI, you can customize this in your adapter. Something like:

registerInteractionHandler: (evtType: string, handler: EventListener) => {
  const target = evtType === 'mouseup' || evtType === 'pointerup' ? 'window' : this.element
  this.listen(target, evtType, handler)
},
deregisterInteractionHandler: (evtType: string, handler: EventListener) => {
  const target = evtType === 'mouseup' || evtType === 'pointerup' ? 'window' : this.element
  this.unlisten(target, handler)
},

@JulianGaibler
Copy link

@mpiroc Did you try this? I just did and had no luck.
If you tried this (and it worked) would you mind elaborating on how you did it?

@mpiroc
Copy link

mpiroc commented Oct 23, 2017

@JulianGaibler
Copy link

@mpiroc Sorry for coming up again!!! I looked at that for a while. Before i just used MDCRipple.attachTo(el), so this is quite an advancement for me.

I came this far...uhh, how exactly do i attach an element now or even...initiate it?

new MDCRippleFoundation({
  registerInteractionHandler: (evtType, handler) => {
     const target = evtType === 'mouseup' || evtType === 'pointerup' ? 'window' : this.element
    this.listen(target, evtType, handler)
  },
  deregisterInteractionHandler: (evtType, handler) => {
    const target = evtType === 'mouseup' || evtType === 'pointerup' ? 'window' : this.element
    this.unlisten(target, handler)
  }
})

Would be really nice if you could explain this. My only alternative is @theenoahmason's way..

@mpiroc
Copy link

mpiroc commented Oct 23, 2017

Hi @JulianWels, it sounds like you're using the default component implementations. My implementation is specifically for the Angular 2 framework.

Since the problem is in the default adapter (see MDCRipple.createAdapter source), you need to provide your own adapter. I suggest you replace const ripple = MDCRipple.attachTo(el) with something like:

const adapter = MDCRipple.createAdapter(/* RippleCapableSurface instance */)
adapter.registerInteractionHandler = (evtType, handler) => {
    // your own implementation, see MDCRipple.createAdapter source for reference
}
adapter.deregisterInteractionHandle = (evtType, handler) => {
    // your own implementation, see MDCRipple.createAdapter source for reference
}
const foundation = new MDCFoundation(adapter)
const ripple = new MDCRipple(el, foundation)

Note that I haven't used the default component implementations, so this might not be the recommended way.

@mpiroc
Copy link

mpiroc commented Oct 23, 2017

By the way, IMO the behavior in the default adapter is a bug and should be fixed. My snippet just overrides the (incorrect IMO) default behavior.

@touficbatache
Copy link
Contributor

touficbatache commented Oct 24, 2017

Hi @mpiroc,
Your solution is awesome! Do you have another one for ripples activating when they are being touched (accidently) when scrolling on touch devices? It's very annoying, when you scroll you have all ripple elements activated...
Thanks for your help!

@djensen47
Copy link

djensen47 commented Nov 15, 2017

Agreed, I also think this is a bug. It's actually quite pervasive throughout the platform. List items will do the same thing. It's especially bad with a tap and scroll on a list item.

@Slion
Copy link

Slion commented Nov 16, 2021

I landed here after looking for a way to remove focus highlights on switches and buttons.
My solution: is to define the following in your CSS and include it below MDC's own CSS:

:root {
    /* We don't want focus highlight on text buttons */
    --mdc-text-button-focus-state-layer-opacity: 0.0;
    /* We don't want focus highlight on switches */
    --mdc-switch-unselected-focus-state-layer-opacity: 0.0;
    --mdc-switch-selected-focus-state-layer-opacity: 0.0;
    --mdc-switch-unselected-pressed-state-layer-opacity: 0.0;
    --mdc-switch-selected-pressed-state-layer-opacity: 0.0;
}

There are a bunch of such custom properties you can override to customize the look and feel of MDC.

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