[Vertical Tabs] Sets action button min touch target to 48dpx48dp [chromium/src : main]
Joel Tan-Aristy (Gerrit)
Joel Tan-Aristy voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Related details
- Gazal Agarwal
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gazal Agarwal (Gerrit)
Gazal Agarwal added 3 comments![]( "Open in Gerrit")
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())) {This includes AL as well (use `isTablet && !isDesktop`).
Let's wait on Julia to confirm the size of the button.
actionButton.getHitRect(rect);
int minTouchTargetPx =
view.getResources().getDimensionPixelSize(R.dimen.min_touch_target_size);
if (rect.width() < minTouchTargetPx) {
int deltaX = (minTouchTargetPx - rect.width()) / 2;
rect.left -= deltaX;
rect.right += deltaX;
}
if (rect.height() < minTouchTargetPx) {
int deltaY = (minTouchTargetPx - rect.height()) / 2;
rect.top -= deltaY;
rect.bottom += deltaY;
}
view.setTouchDelegate(new TouchDelegate(rect, actionButton));We should defer this until after layout, by wrapping it in `view.post(...)`.
Would also be good to check actionButton.isAttachedToWindow() and actionButton.getVisibility() before setting the touch delegate.
android.content.res.Configuration config = mActivity.getResources().getConfiguration();inline fqn
Related details
- Joel Tan-Aristy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Tan-Aristy (Gerrit)
Joel Tan-Aristy voted and added 3 comments![]( "Open in Gerrit")
Votes added by Joel Tan-Aristy
| Auto-Submit | +0 |
| Commit-Queue | +1 |
3 comments
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())) {This includes AL as well (use `isTablet && !isDesktop`).
Let's wait on Julia to confirm the size of the button.
Added Checks for isTablet && !isDesktop. Will hold this cl until confirmation from Julia is recieved
actionButton.getHitRect(rect);
int minTouchTargetPx =
view.getResources().getDimensionPixelSize(R.dimen.min_touch_target_size);
if (rect.width() < minTouchTargetPx) {
int deltaX = (minTouchTargetPx - rect.width()) / 2;
rect.left -= deltaX;
rect.right += deltaX;
}
if (rect.height() < minTouchTargetPx) {
int deltaY = (minTouchTargetPx - rect.height()) / 2;
rect.top -= deltaY;
rect.bottom += deltaY;
}
view.setTouchDelegate(new TouchDelegate(rect, actionButton));We should defer this until after layout, by wrapping it in `view.post(...)`.
Would also be good to check actionButton.isAttachedToWindow() and actionButton.getVisibility() before setting the touch delegate.
Added the post and additional checks.
android.content.res.Configuration config = mActivity.getResources().getConfiguration();Joel Tan-Aristyinline fqn
Done
Related details
- Gazal Agarwal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Tan-Aristy (Gerrit)
Joel Tan-Aristy added 1 comment![]( "Open in Gerrit")
- Gazal Agarwal
- Neil Coronado
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Tan-Aristy (Gerrit)
Joel Tan-Aristy voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joel Tan-Aristy
| Auto-Submit | +1 |
1 comment
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())) {Joel Tan-AristyThis includes AL as well (use `isTablet && !isDesktop`).
Let's wait on Julia to confirm the size of the button.
Added Checks for isTablet && !isDesktop. Will hold this cl until confirmation from Julia is recieved
Confirmed with Julia, 40x48!
Related details
- Gazal Agarwal
- Neil Coronado
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neil Coronado (Gerrit)
Neil Coronado voted and added 3 comments![]( "Open in Gerrit")
Votes added by Neil Coronado
| Code-Review | +1 |
3 comments
<dimen name="vertical_tab_action_button_touch_target_height">40dp</dimen>dimens lgtm, but should the description say the min touch target is 48x40dp?
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())) {Joel Tan-AristyThis includes AL as well (use `isTablet && !isDesktop`).
Let's wait on Julia to confirm the size of the button.
Joel Tan-AristyAdded Checks for isTablet && !isDesktop. Will hold this cl until confirmation from Julia is recieved
Confirmed with Julia, 40x48!
I'm a bit surprised we're using 48x40dp here.
we use this touch target size for the horizontal tab strip, but that's considered an edge case, since the horizontal tab strip itself is only 40dp tall, and it's non-trivial to increase its height (tl;dr it'd result in less webcontents space, potentially affecting search metrics, etc.).
the horizontal tab strip was given an exception to do this, since there's alternatives to its actions that _are_ a11y-compliant (in the GTS, etc.).
I wouldn't block on this, but I would have assumed that we _could_ support a true 48x48dp target for VT, given that it doesn't have the same height constraints (though I'm not sure how easy or hard that would be to support... e.g. if we have to actually reposition/resize views). If that _is_ the case, then maybe we add a TODO to revisit.
view.setTouchDelegate(null);do we need to reset the touch delegate when the view is invisible? i.e. does the touch delegate not get ignored when the view is invisible?
Related details
- Gazal Agarwal
- Joel Tan-Aristy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gazal Agarwal (Gerrit)
Gazal Agarwal added 1 comment![]( "Open in Gerrit")
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())) {Joel Tan-AristyThis includes AL as well (use `isTablet && !isDesktop`).
Let's wait on Julia to confirm the size of the button.
Added Checks for isTablet && !isDesktop. Will hold this cl until confirmation from Julia is recieved
Joel: Forgot to update the check to tablet only?
Neil: The min height for VT tabs on tablets will be 40dp as well. I'm guessing that's the reason behind this number.
Related details
- Joel Tan-Aristy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |