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

style(ripple): start fade out on mouseup #7397

Closed
wants to merge 2 commits into from

Conversation

willshowell
Copy link
Contributor

Purpose

The goal of this is to start the ripple fade out immediately after mouseup rather than having to wait until the ripple animation has completed. My intention was to match the behavior seen in md1 which feels snappier and cleaner.

Background

The change to make sure a ripple had completed its animation before hiding was made in #3400. It looks like the intention was to solve the problem of ripples not being added to the active ripple set until after it was completed, as pointed out in #3398. However, it appears #3400 fixed that problem anyway by adding it to the active ripple set as soon as animation begins. Therefore, the additional check of making sure a ripple had completed its animation is not necessary.

Before

ripple-b

After

ripple - a

This is one that's been a nit of mine for a while, but I couldn't tell if the change a while back was intentional or accidental. @devversion, thoughts? If this ok, I'll add tests.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 28, 2017
@devversion
Copy link
Member

devversion commented Sep 29, 2017

I'm not sure which behavior is preferred here. For me (personally) it feels nicer if the ripples are finishing their animation. This is can be seen on normal mat-button elements in the demo-app.

When looking at MDC, we can see that the ripples are also finishing their animation before starting to fade out.

In general, MDC varies in the implementation of the ripples, but a few aspects from the UX seem to be common enough to follow over all those implementations. I'm up for more discussion about this.

@willshowell
Copy link
Contributor Author

Yeah I suppose it's very much a personal preference. My concern when the current approach is that it feels quite piece-wise, where the expanding and fadeout are two distinctly separate animations. I'd prefer a smooth blending of the two.

When I look at these examples from the spec, it feels like the ripples are starting to fade out before reaching the boundaries. It can be seen best in this animation. That said, there is a difference between

  • Start fading out on mouseup
  • Lower opacity as radius grows, to mimic ink spreading thinner

The second option is really what I'd prefer - not sure of the complexity.

@willshowell
Copy link
Contributor Author

I guess I'll also add,

One of the things that triggered me thinking about this was this SO question, and the link they provide to material-ui. I can't help but prefer the smooth ink animations on the radio buttons, icon buttons, menu items, etc.

@ghost
Copy link

ghost commented Oct 2, 2017

After a look on the MD guidelines/Buttons / Behavior, the buttons should have something like a fade out animation.

@willshowell
Copy link
Contributor Author

I see Google's calendar refresh uses buttons in line with the existing behavior. Maybe I'm behind on the times 😄.

I still want to play with this further and see if there's some tweaking left to do... maybe using radial-gradients in the ripple? or a different easing curve for opacity so there's not such a sharp distinction in the two transitions. Will report back.

@willshowell
Copy link
Contributor Author

@devversion thanks for keeping this open. I've tried adjusting the curves so that opacity and transform use opposing curves.

Transform still uses an ease-out curve. But once complete, opacity uses an ease-in curve. The idea being that the transition seamlessly moves from transform to opacity, without the sudden switch in velocity. I don't really know how else to explain it, so I've drawn a diagram 😄

I know the difference is subtle, but I do feel like it's an improvement.

Before
ripple-before

After
ripple-after

@devversion
Copy link
Member

devversion commented Nov 11, 2017

@willshowell No problem, thanks for your work on that detail. It's really hard to make sure we are following the exact specification in that case. The specifications for the ripple are very dry and don't go into depth about details like that.

I'm not really sure which behavior is the right one, but I feel like in the specification videos from the Material Design guidelines the fade-out animation feels faster, and more aggressive than the one in your GIF.

https://material.io/guidelines/motion/material-motion.html#material-motion-how-does-material-move

I'm really up for discussion about this

@ghost
Copy link

ghost commented Nov 13, 2017

Hmmmmm I'm still of the opinion that the Ink Ripple doesn't always reach the frame of the button, just have a look on this animation and keep your eyes on the corners of the buttons, if the point of touch is at the opposite edge, the Ripple fades out before it reaches the end of the buttons. But this behavior seems only to exist, when the touch is a short one, I guess.

However, I agree with @devversion, the animations in the specs really feels faster.

@jelbourn
Copy link
Member

@willshowell @devversion revisiting this- our ripple should follow the behavior of MDC web. What do they do?

@willshowell
Copy link
Contributor Author

@devversion I'll let you decide if you want to keep this open for discussion. I think #9253 will allow for an appropriate amount of flexibility, but perhaps in the future RippleGlobalOptions could have a "terminate ripple on mouseup" option...

@devversion
Copy link
Member

devversion commented Jan 24, 2018

@willshowell As per MDC, the ripples should fade-out once the fade-in animation of the ripples completed.

This behavior is kind of different from implementation to implementation, and therefore it's hard to properly judge. However, MDC uses the same concept, and it kind of feels more friendly (IMO)

In regards to the option, I agree that it would be nice having such an option (terminatOnMouseup). Can you file an issue for discussion about this?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
@mmalerba mmalerba added the needs: discussion Further discussion with the team is needed before proceeding label Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement needs: discussion Further discussion with the team is needed before proceeding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants