Closed Bug 1396076 Opened 2 years ago Closed 2 years ago

[a11y] Tabs tray close tab button is very small and potentially hard to press

Categories

(Firefox for Android :: Theme and Visual Design, enhancement, P1)

All
Android
enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(4 files)

+++ This bug was initially created as a clone of Bug #1390670 +++

We already extend the touch target area of the close button to 40x40dp using a TouchDelegate, and we can implement a HoverDelegate along the same lines to increase the target area for "touch exploration" as well.

Therefore, I'd prefer implementing this rather than messing around with the padding, which has the drawback of impinging on the space left for displaying the tab title.
Comment on attachment 8903910 [details]
Bug 1396076 - Part 1 - Add a HoverDelegate class.

https://reviewboard.mozilla.org/r/175558/#review181638

::: mobile/android/base/java/org/mozilla/gecko/widget/HoverDelegateWithReset.java:79
(Diff revision 2)
>          mSlopBounds = new Rect(bounds);
>          mSlopBounds.inset(-mSlop, -mSlop);
>          mDelegateView = delegateView;
> +
> +        try {
> +            mDispatchHover = View.class.getDeclaredMethod("dispatchHoverEvent", MotionEvent.class);

The comments of `dispatchHoverEvent` in View.java said we shouldn't call it directly, call `dispatchGenericMotionEvent(MotionEvent)` instead.

Is there any reason we don't use dispatchGenericMotionEvent?

http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.1.1_r1/android/view/View.java#8590
Comment on attachment 8903910 [details]
Bug 1396076 - Part 1 - Add a HoverDelegate class.

https://reviewboard.mozilla.org/r/175558/#review181638

> The comments of `dispatchHoverEvent` in View.java said we shouldn't call it directly, call `dispatchGenericMotionEvent(MotionEvent)` instead.
> 
> Is there any reason we don't use dispatchGenericMotionEvent?
> 
> http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.1.1_r1/android/view/View.java#8590

Reading more closely through the source I think you're right - thanks for the reminder.
I've also discovered a small issue with the existing code for the close button: Using the OnPreDrawListener, when rotating the tabs tray from portrait mode to landscape the bounds of the hit rectangle are miscalculated for some of the tabs.
Comment on attachment 8903910 [details]
Bug 1396076 - Part 1 - Add a HoverDelegate class.

https://reviewboard.mozilla.org/r/175558/#review182632
Attachment #8903910 - Flags: review?(topwu.tw) → review+
Comment on attachment 8903911 [details]
Bug 1396076 - Part 2 - Use the HoverDelegate for TabsLayoutItemView's close button.

https://reviewboard.mozilla.org/r/175560/#review182638
Attachment #8903911 - Flags: review?(topwu.tw) → review+
Comment on attachment 8905672 [details]
Bug 1396076 - Part 3 - Switch to a LayoutChangeListener for setting up the delegates.

https://reviewboard.mozilla.org/r/177456/#review182642
Attachment #8905672 - Flags: review?(topwu.tw) → review+
Comment on attachment 8903912 [details]
Bug 1396076 - Part 4 - Revert TabsLayoutItemView padding changes for the "close tab" button.

https://reviewboard.mozilla.org/r/175562/#review182646
Attachment #8903912 - Flags: review?(topwu.tw) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/d57e40a922ee
Part 1 - Add a HoverDelegate class. r=jwu
https://hg.mozilla.org/integration/autoland/rev/bc9e3b8c4100
Part 2 - Use the HoverDelegate for TabsLayoutItemView's close button. r=jwu
https://hg.mozilla.org/integration/autoland/rev/60f7b76c8f7f
Part 3 - Switch to a LayoutChangeListener for setting up the delegates. r=jwu
https://hg.mozilla.org/integration/autoland/rev/e5e5c095a5eb
Part 4 - Revert TabsLayoutItemView padding changes for the "close tab" button. r=jwu
You need to log in before you can comment on or make changes to this bug.