New issue
Advanced search Search tips

Issue 686962 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocked on:
issue 674269
issue 691895



Sign in to add a comment

Harmony close buttons should be 16 DIP/side, not 24

Project Member Reported by pkasting@chromium.org, Jan 31 2017

Issue description

Current Harmony close buttons are 24 DIP per side (since they are VectorIconButtons, and all such buttons are 24 DIP).  The spec says to use 16.

Open questions for Alan:
* Should other such icon buttons also be 16?  For example, the find next/previous "arrow" buttons in the find box.  My patch to fix this changes these as well.  This seems likely correct to me (it would be weird to have three icons in a row, but the third has a smaller target), but I'm a little leery of the sort of fixed-size assumptions I'm seeing in the code.

* Playing with these close buttons for only a few minutes, the hover targets feel *really* small at 16 px.  I'm worried about a11y here in particular.  Compare to e.g. Windows native where window close button targets are humongous (~45x29 on Win 10), we're going from something that's about 1/2 the window close button area to something that's about 1/5th the close button area.  It feels a little tricky to target to me.  How much practical testing have we already done here to feel confident of this choice?  What sort of testing/considerations should we give as/after we land, to make sure we stay happy with it?
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 31 2017

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

commit 233a48eb42ca4b36a788c7d366a438ff4066015b
Author: pkasting <pkasting@chromium.org>
Date: Tue Jan 31 03:36:57 2017

Allow ripple hover/press effects to be more easily sized by subclasses.

This will be useful in the harmony work, where we want icon buttons to have a
ripple size that matches the button size, rather than using the fixed 24 px
size.

This also converts some consts to constexprs -- I needed to move one constant to
the header for my default argument values, and having it constexpr and the
things above it const looked weird.

BUG= 686962 
TEST=none

Review-Url: https://codereview.chromium.org/2665073002
Cr-Commit-Position: refs/heads/master@{#447178}

[modify] https://crrev.com/233a48eb42ca4b36a788c7d366a438ff4066015b/ui/views/animation/ink_drop_host_view.cc
[modify] https://crrev.com/233a48eb42ca4b36a788c7d366a438ff4066015b/ui/views/animation/ink_drop_host_view.h

16px is too small to be a reasonable click target, I think. We should bring this up on the chrome-harmony list.
Labels: -Pri-3 Pri-1
Raising to P1, because even if we decide this size works for mouse, we may want something different for touch.  That sort of thing should be determined now so we have time to architect correctly.
Owner: bettes@chromium.org
->bettes to at least answer the stuff in comment 0 and to consider a11y and touch in general.
Blockedon: 691895
Blockedon: 674269
Status: WontFix (was: Assigned)
The spec here is now that close buttons should be 24 DIP.

Sign in to add a comment