New issue
Advanced search Search tips

Issue 864430 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Feature
Q2



Sign in to add a comment

3D touch accelerator for long-press menus on the new toolbar

Project Member Reported by stkhapugin@chromium.org, Jul 17

Issue description

The new toolbar has cool additional actions available on long press. But long press is long, and life is short - why wait? In this age of instant gratification, could we let users trade a little more force for a little bit of time in their life by adding a force-touch gesture recognizer that duplicates the menu gesture? And a little haptic feedback for reassuring the user? 
 
In the past I was against this mainly because I was thinking of using *either* 3D Touch or longpress (https://www.google.com/url?q=https://bugs.chromium.org/p/chromium/issues/detail?id%3D821560%23c10&sa=D&source=hangouts&ust=1531912313669000&usg=AFQjCNFWleFztnSyKKWRxAbeluVpKjdBPw).

I'm fine with hooking up both gestures as stk suggests, however, as long as they don't collide in weird ways with one another.
Cc: pschaffner@chromium.org
Labels: -Pri-3 Q2 Pri-1
Owner: gambard@chromium.org
One "weird way" things could collide is a 3D Touch triggering when an tap was intended (something that might effect those heavy-tappers out there). For this reason I suggested using the maximum amount of force to trigger the 3D Touch gesture.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a809279461448ac3d28204f2b7af2d988e60107d

commit a809279461448ac3d28204f2b7af2d988e60107d
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Jul 20 09:19:10 2018

Create a ForceTouchRecognizer for PopupMenu

This CL creates a custom GestureRecognizer triggerring on force touches.
This CL is using this gesture recognizer to trigger the popup menu on
force touch events.
It also modifies the HapticFeedback interface to allow the impact
feedback to specify its style.

Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9f7c03a2404fdf066c1f870e116eb9c143f387d3
Reviewed-on: https://chromium-review.googlesource.com/1143191
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576817}
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.mm
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/side_swipe/side_swipe_navigation_view.mm
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/tabs/tab_strip_controller.mm
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller.mm
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/uikit_ui_util.h
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/uikit_ui_util.mm
[modify] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/util/BUILD.gn
[add] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer.h
[add] https://crrev.com/a809279461448ac3d28204f2b7af2d988e60107d/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer.mm

Labels: Merge-Request-69
Status: Fixed (was: Assigned)
Project Member

Comment 6 by sheriffbot@chromium.org, Jul 21

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kariahda@chromium.org
+kariahda@ for merge approval.
Is it possible to get unit tests for this?
What do you mean? You want unit tests and to cherry pick them?
If it's possible to add or update unit tests, please do. They do not need to be cherry-picked, but we do want to make sure they're passing. There are many merges happening for M69, so we want to make sure M69 is as stable as it can be.
Project Member

Comment 11 by sheriffbot@chromium.org, Jul 24

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Unit test under review: crrev.com/c/1150142

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e630f950e1d7e3ad796004e791e0a6d30ee4adb4

commit e630f950e1d7e3ad796004e791e0a6d30ee4adb4
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Jul 26 17:24:51 2018

Add unit test for toolbars' force touch gestures

This CL adds a unit test for the force touch gesture recognizer added
to the toolbar buttons, triggering the popup menu.

Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Icc2524f7e8e58bf20fb4b1381913cb82c57206a8
Reviewed-on: https://chromium-review.googlesource.com/1150142
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578349}
[modify] https://crrev.com/e630f950e1d7e3ad796004e791e0a6d30ee4adb4/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[add] https://crrev.com/e630f950e1d7e3ad796004e791e0a6d30ee4adb4/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller_unittest.mm
[modify] https://crrev.com/e630f950e1d7e3ad796004e791e0a6d30ee4adb4/ios/chrome/browser/ui/util/BUILD.gn
[add] https://crrev.com/e630f950e1d7e3ad796004e791e0a6d30ee4adb4/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer_unittest.mm
[modify] https://crrev.com/e630f950e1d7e3ad796004e791e0a6d30ee4adb4/ios/chrome/test/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9d977bf4beb462f038cc1b55220cc0e3e986eb1

commit c9d977bf4beb462f038cc1b55220cc0e3e986eb1
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Jul 26 18:40:17 2018

Revert "Add unit test for toolbars' force touch gestures"

This reverts commit e630f950e1d7e3ad796004e791e0a6d30ee4adb4.

Reason for revert: AdaptiveToolbarViewControllerTest.DetectForceTouch failing on ios-uirefresh-simulator

Bug:  868017 

Original change's description:
> Add unit test for toolbars' force touch gestures
> 
> This CL adds a unit test for the force touch gesture recognizer added
> to the toolbar buttons, triggering the popup menu.
> 
> Bug:  864430 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: Icc2524f7e8e58bf20fb4b1381913cb82c57206a8
> Reviewed-on: https://chromium-review.googlesource.com/1150142
> Reviewed-by: Mark Cogan <marq@chromium.org>
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578349}

TBR=marq@chromium.org,gambard@chromium.org

Change-Id: I7d4833a56c139ce501c673cd305c19efa456e1e0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1151927
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578381}
[modify] https://crrev.com/c9d977bf4beb462f038cc1b55220cc0e3e986eb1/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[delete] https://crrev.com/48d9716bcc72f5899be15244c35297c363ab41e0/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller_unittest.mm
[modify] https://crrev.com/c9d977bf4beb462f038cc1b55220cc0e3e986eb1/ios/chrome/browser/ui/util/BUILD.gn
[delete] https://crrev.com/48d9716bcc72f5899be15244c35297c363ab41e0/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer_unittest.mm
[modify] https://crrev.com/c9d977bf4beb462f038cc1b55220cc0e3e986eb1/ios/chrome/test/BUILD.gn

Project Member

Comment 15 by sheriffbot@chromium.org, Jul 27

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -Merge-Approved-69 Merge-TBD
Changing to merge-TBD. Please re-request when ready.
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b49cfc48e05e838d1e7b43d6d577603ca27a37e

commit 7b49cfc48e05e838d1e7b43d6d577603ca27a37e
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Jul 30 15:21:48 2018

Reland "Add unit test for toolbars' force touch gestures"

This CL adds a unit test for the force touch gesture recognizer added
to the toolbar buttons, triggering the popup menu.

Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I756b16fea096cda8b1f3f1e563236d12e2d11e34
Reviewed-on: https://chromium-review.googlesource.com/1152926
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579043}
[modify] https://crrev.com/7b49cfc48e05e838d1e7b43d6d577603ca27a37e/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[add] https://crrev.com/7b49cfc48e05e838d1e7b43d6d577603ca27a37e/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller_unittest.mm
[modify] https://crrev.com/7b49cfc48e05e838d1e7b43d6d577603ca27a37e/ios/chrome/browser/ui/util/BUILD.gn
[add] https://crrev.com/7b49cfc48e05e838d1e7b43d6d577603ca27a37e/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer_unittest.mm
[modify] https://crrev.com/7b49cfc48e05e838d1e7b43d6d577603ca27a37e/ios/chrome/test/BUILD.gn

Labels: -Merge-TBD Merge-Request-69
Test has been added, merge request for https://chromium-review.googlesource.com/1143191.
Project Member

Comment 19 by sheriffbot@chromium.org, Jul 30

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thank you for unit tests! I should have mentioned this earlier but let's get canary verification as well. This can by done by developer.
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adaabea92f7531e36a4ad4b1bf435ae94c53026c

commit adaabea92f7531e36a4ad4b1bf435ae94c53026c
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Mon Jul 30 17:25:45 2018

Revert "Reland "Add unit test for toolbars' force touch gestures""

This reverts commit 7b49cfc48e05e838d1e7b43d6d577603ca27a37e.

Reason for revert: AdaptiveToolbarViewControllerTest.DetectForceTouch and ForceTouchLongPressGestureRecognizerTest.DetectForceTouch crash when running on iPhone 5 iOS 10.3 simulator.

Original change's description:
> Reland "Add unit test for toolbars' force touch gestures"
> 
> This CL adds a unit test for the force touch gesture recognizer added
> to the toolbar buttons, triggering the popup menu.
> 
> Bug:  864430 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I756b16fea096cda8b1f3f1e563236d12e2d11e34
> Reviewed-on: https://chromium-review.googlesource.com/1152926
> Reviewed-by: Mark Cogan <marq@chromium.org>
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#579043}

TBR=marq@chromium.org,gambard@chromium.org

Change-Id: I890f1e44f3aeb92a3258df1b372a0901cb5251c6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1155052
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579076}
[modify] https://crrev.com/adaabea92f7531e36a4ad4b1bf435ae94c53026c/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[delete] https://crrev.com/1e4639d7ac2bc1fdd25427f4df7239906d5c918d/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller_unittest.mm
[modify] https://crrev.com/adaabea92f7531e36a4ad4b1bf435ae94c53026c/ios/chrome/browser/ui/util/BUILD.gn
[delete] https://crrev.com/1e4639d7ac2bc1fdd25427f4df7239906d5c918d/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer_unittest.mm
[modify] https://crrev.com/adaabea92f7531e36a4ad4b1bf435ae94c53026c/ios/chrome/test/BUILD.gn

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9fa36abba61799ebe706e62c8575d1e65392e37b

commit 9fa36abba61799ebe706e62c8575d1e65392e37b
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Jul 31 09:21:58 2018

Reland "Add unit test for toolbars' force touch gestures"

This CL adds a unit test for the force touch gesture recognizer added
to the toolbar buttons, triggering the popup menu.

TBR=marq@chromium.org

Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ia87d675b460922eb1991de4022438e0ed0b1ce4d
Reviewed-on: https://chromium-review.googlesource.com/1156304
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579356}
[modify] https://crrev.com/9fa36abba61799ebe706e62c8575d1e65392e37b/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[add] https://crrev.com/9fa36abba61799ebe706e62c8575d1e65392e37b/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller_unittest.mm
[modify] https://crrev.com/9fa36abba61799ebe706e62c8575d1e65392e37b/ios/chrome/browser/ui/util/BUILD.gn
[add] https://crrev.com/9fa36abba61799ebe706e62c8575d1e65392e37b/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer_unittest.mm
[modify] https://crrev.com/9fa36abba61799ebe706e62c8575d1e65392e37b/ios/chrome/test/BUILD.gn

Status: Verified (was: Fixed)
Labels: -Hotlist-Merge-Review -Merge-Review-69 Merge-Approved-69
Approved.
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 31

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12fdac29f185cd51836cc832ace8218b539482ff

commit 12fdac29f185cd51836cc832ace8218b539482ff
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Jul 31 16:01:19 2018

Create a ForceTouchRecognizer for PopupMenu

This CL creates a custom GestureRecognizer triggerring on force touches.
This CL is using this gesture recognizer to trigger the popup menu on
force touch events.
It also modifies the HapticFeedback interface to allow the impact
feedback to specify its style.

Bug:  864430 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9f7c03a2404fdf066c1f870e116eb9c143f387d3
Reviewed-on: https://chromium-review.googlesource.com/1143191
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#576817}(cherry picked from commit a809279461448ac3d28204f2b7af2d988e60107d)
Reviewed-on: https://chromium-review.googlesource.com/1156725
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#274}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.mm
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/side_swipe/side_swipe_navigation_view.mm
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/tabs/tab_strip_controller.mm
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller.mm
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/uikit_ui_util.h
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/uikit_ui_util.mm
[modify] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/util/BUILD.gn
[add] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer.h
[add] https://crrev.com/12fdac29f185cd51836cc832ace8218b539482ff/ios/chrome/browser/ui/util/force_touch_long_press_gesture_recognizer.mm

Sign in to add a comment