Ripple animations should not be shown on touch input for Windows |
||||||||||||
Issue descriptionWindows has its own OS-level feedback effect on touch input, so do not show the material design ripple effects on buttons in response to touch.
,
May 4 2016
,
May 4 2016
I started investigating this last night, stay tuned for findings.
,
May 4 2016
,
Jun 20 2016
Now that https://codereview.chromium.org/2042073002/ has landed it should be much easier to complete this issue. All ripple animations now go through InkDropHostView::AnimateInkDrop() which accepts a 'const ui::LocatedEvent*' parameter. So it should be easy to use an '#if defined(OS_WIN)' and suppress ripple animations based on the |event| parameter. The tricky part is going to be making sure the ripples still make valid state transitions with the introduction of gesture events not triggering animations. e.g. 1. User presses and holds left mouse button on 'Home' button => ripple animates to ACTION_PENDING 2. User taps the 'Home' button => triggers Home (this should definitely make the ripple disappear, probably via a ACTION_TRIGGERED animation)
,
Jun 22 2016
,
Jun 23 2016
Could it really be as simple as adding this to InkDropHostView::AnimateInkDrop()?
#if defined(OS_WIN)
if (state != InkDropState::HIDDEN && event && event->IsGestureEvent())
return;
#endif
I did this and tried the steps in #5 and it seems to work as intended.
,
Jun 23 2016
I think it will be slightly more complex than that. Some examples to think through are: 1. The ACTIVATED animation should occur no matter what the event type is. 2. An ACTION_TRIGGERED, ALT_ACTION_TRIGGERED and DEACTIVATED animation triggered by a gesture event should probably happen if the ripple is already visible. 3. And we should try and make sure ripples are robust when multiple input sources are interacting with the button. This is probably a small use case but leaving a ripple persist is a pretty bad user experience.
,
Jun 23 2016
Ok, after looking over the available InkDropStates and your comment in #8, it seems that we really only want to block any "pending" actions from touch gestures. The rest of them should go ahead and pass on through.
#if defined(OS_WIN)
if ((state == InkDropState::PENDING ||
state == InkDropState::ALTERNATE_ACTION_PENDING) && event &&
event->IsGestureEvent())
return;
#endif
,
Jun 23 2016
When I do that, it looks odd. I see the remnants of the OS ink-drop and the action animation... This is on a Surface Book with Windows 10. I'll see if I can make a short video and post it.
,
Jun 23 2016
The built-in screen/app recorder in Win10 doesn't capture the OS ink-drops... only the ink-drops we paint. This makes it hard to show what I'm seeing here.
,
Jun 23 2016
Maybe high frame rate recording with a phone might work.
,
Jun 23 2016
Certainly a little more complicated, but this seems to work.
#if defined(OS_WIN)
if ((state == InkDropState::ACTION_PENDING ||
state == InkDropState::ALTERNATE_ACTION_PENDING ||
((state == InkDropState::ACTION_TRIGGERED ||
state == InkDropState::ALTERNATE_ACTION_TRIGGERED) &&
ink_drop_->GetTargetInkDropState() == InkDropState::HIDDEN)) &&
event && event->IsGestureEvent())
return;
#endif
,
Jun 27 2016
When it lands, can you also verify that your change papers over bug 615013 ?
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d480e04cbde45c22342599468c3081f586b4523c commit d480e04cbde45c22342599468c3081f586b4523c Author: kylixrd <kylixrd@chromium.org> Date: Mon Jun 27 18:48:36 2016 Don't show ink drops with touch events on Windows BUG= 595315 TBR=bruthig@chromium.org Review-Url: https://codereview.chromium.org/2095873004 Cr-Commit-Position: refs/heads/master@{#402239} [modify] https://crrev.com/d480e04cbde45c22342599468c3081f586b4523c/ui/views/animation/ink_drop_host_view.cc
,
Jun 28 2016
Just verified with today's Canary build on my Surface Book. No highlight remains or is shown for a long-tap or touch.
,
Jun 28 2016
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fff99399f7fcfe17950174897fb47a26f6047c78 commit fff99399f7fcfe17950174897fb47a26f6047c78 Author: kylixrd <kylixrd@chromium.org> Date: Fri Jul 01 16:05:01 2016 Added tests and reworked ink-drop touch logic BUG= 595315 Review-Url: https://codereview.chromium.org/2117603002 Cr-Commit-Position: refs/heads/master@{#403462} [modify] https://crrev.com/fff99399f7fcfe17950174897fb47a26f6047c78/ui/views/animation/ink_drop_host_view.cc [modify] https://crrev.com/fff99399f7fcfe17950174897fb47a26f6047c78/ui/views/animation/ink_drop_host_view_unittest.cc
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82397d9c36b76e2869ab77ede2df736871c752b2 commit 82397d9c36b76e2869ab77ede2df736871c752b2 Author: kelvinp <kelvinp@chromium.org> Date: Fri Jul 01 16:32:16 2016 Revert of Added tests and reworked ink-drop touch logic (patchset #2 id:20001 of https://codereview.chromium.org/2117603002/ ) Reason for revert: Compilation failure on Chromium.win builder. https://build.chromium.org/p/chromium.win/builders/Win%20x64%20Builder/builds/22983/steps/compile/logs/stdio Original issue's description: > Added tests and reworked ink-drop touch logic > > BUG= 595315 > > Committed: https://crrev.com/fff99399f7fcfe17950174897fb47a26f6047c78 > Cr-Commit-Position: refs/heads/master@{#403462} TBR=bruthig@chromium.org,kylixrd@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 595315 Review-Url: https://codereview.chromium.org/2115933002 Cr-Commit-Position: refs/heads/master@{#403466} [modify] https://crrev.com/82397d9c36b76e2869ab77ede2df736871c752b2/ui/views/animation/ink_drop_host_view.cc [modify] https://crrev.com/82397d9c36b76e2869ab77ede2df736871c752b2/ui/views/animation/ink_drop_host_view_unittest.cc
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2492af4d898c22ca3a341e67cb223e0e7ebabe0c commit 2492af4d898c22ca3a341e67cb223e0e7ebabe0c Author: kylixrd <kylixrd@chromium.org> Date: Fri Jul 01 18:02:28 2016 Added tests and reworked ink-drop touch logic BUG= 595315 Committed: https://crrev.com/fff99399f7fcfe17950174897fb47a26f6047c78 Review-Url: https://codereview.chromium.org/2117603002 Cr-Original-Commit-Position: refs/heads/master@{#403462} Cr-Commit-Position: refs/heads/master@{#403481} [modify] https://crrev.com/2492af4d898c22ca3a341e67cb223e0e7ebabe0c/ui/views/animation/ink_drop_host_view.cc [modify] https://crrev.com/2492af4d898c22ca3a341e67cb223e0e7ebabe0c/ui/views/animation/ink_drop_host_view_unittest.cc
,
Jul 6 2016
,
Jul 6 2016
,
Jul 6 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95e3c7a8876a78816484f446682b2ec8d87d3760 commit 95e3c7a8876a78816484f446682b2ec8d87d3760 Author: Ben Ruthig <bruthig@chromium.org> Date: Thu Jul 07 18:11:24 2016 Added tests and reworked ink-drop touch logic BUG= 595315 Review-Url: https://codereview.chromium.org/2117603002 Cr-Commit-Position: refs/heads/master@{#403462} (cherry picked from commit fff99399f7fcfe17950174897fb47a26f6047c78) Review URL: https://codereview.chromium.org/2125353002 . Cr-Commit-Position: refs/branch-heads/2785@{#45} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/95e3c7a8876a78816484f446682b2ec8d87d3760/ui/views/animation/ink_drop_host_view.cc [modify] https://crrev.com/95e3c7a8876a78816484f446682b2ec8d87d3760/ui/views/animation/ink_drop_host_view_unittest.cc
,
Apr 5 2017
,
Apr 5 2017
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by tdander...@chromium.org
, Mar 23 2016