Closed
Bug 1396076
Opened 7 years ago
Closed 7 years ago
[a11y] Tabs tray close tab button is very small and potentially hard to press
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, enhancement, P1)
Tracking
(firefox57 fixed)
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
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
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
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.
Assignee | ||
Comment 9•7 years ago
|
||
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
mozreview-review |
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 15•7 years ago
|
||
mozreview-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 16•7 years ago
|
||
mozreview-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 17•7 years ago
|
||
mozreview-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+
Comment hidden (mozreview-request) |
Comment 19•7 years ago
|
||
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
Comment 20•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d57e40a922ee https://hg.mozilla.org/mozilla-central/rev/bc9e3b8c4100 https://hg.mozilla.org/mozilla-central/rev/60f7b76c8f7f https://hg.mozilla.org/mozilla-central/rev/e5e5c095a5eb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•