New issue
Advanced search Search tips

Issue 788024 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 13
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 783540



Sign in to add a comment

Harmony - update RequestPinView

Project Member Reported by x...@chromium.org, Nov 23 2017

Issue description

Chrome Version: (copy from chrome://version)
OS: (e.g. Win7, OSX 10.9.5, etc...)

See https://folio.googleplex.com/_/preview/1sFARH7Ox4zR2zb8Y6PsgWvl0Nrbxouj6/PinRequestview#%2FScreenshot%202017-11-13%20at%2015.33.42.png%3Fz=width&c=show 
for screenshot and required changes.
 

Comment 1 by x...@chromium.org, Nov 23 2017

Blocking: 630357

Comment 2 by tapted@chromium.org, Nov 23 2017

Blocking: -630357

Comment 3 by tapted@chromium.org, Nov 23 2017

Blocking: 783540
Owner: glevin@chromium.org
Status: Started (was: Untriaged)
Summary: Harmony - update RequestPinView (was: Harmony - update PinRequestview)

Comment 6 by glevin@chromium.org, Apr 14 2018

The first patchset at https://chromium-review.googlesource.com/c/chromium/src/+/1013412 generates the attached dialogs (using --enable-features=SecondaryUiMd).  Please take a look and let me know if they're sufficiently harmonious.
hd_pinreq_valid_v1.png
6.3 KB View Download
hd_pinreq_invalid_v1.png
7.9 KB View Download

Comment 7 by bettes@chromium.org, Apr 20 2018

LGTM. Imbalanced whitespace is noted, but the size of the input box should reflect the nature of the input (PIN). Longer inputs would insinuate some long form text input. 
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 27 2018

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

commit 38ab97cd14cc25f42e382f263d1676c40c4e2ac5
Author: glevin <glevin@chromium.org>
Date: Fri Apr 27 20:13:11 2018

Harmonize RequestPinView dialog

As per  crbug.com/788024  Comment 1 (see mocks):
- Fix error box style
- Dialog width = 448
- Input box width = 200, left aligned
- Additionally: add SPACE between "Invalid PIN" & "Attempts left"

Harmonious nature of the pin-requestin' dialog.

Bug:  788024 
Test: Get yourself into some kinda pin-needin' situation and confirm the
Change-Id: Ie573c8a79fdb2001a3c2b66adc4b80a4b09dd723
Reviewed-on: https://chromium-review.googlesource.com/1013412
Commit-Queue: Greg Levin <glevin@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554472}
[modify] https://crrev.com/38ab97cd14cc25f42e382f263d1676c40c4e2ac5/chrome/browser/chromeos/ui/request_pin_view.cc
[modify] https://crrev.com/38ab97cd14cc25f42e382f263d1676c40c4e2ac5/chrome/browser/chromeos/ui/request_pin_view.h

Comment 9 by glevin@chromium.org, Apr 27 2018

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
glevin@, apologies for not catching this earlier, but the red error text is too light for a11y. The standard hex for red text is #D93025. 

https://docs.google.com/presentation/d/19yQuvKqmWPxe8TloAQ_5C12FV6v9eQ3qIFDXXuF10G8/edit#slide=id.g3b31c5d673_0_24
That looks like kGoogleRed600, defined here:
https://cs.chromium.org/chromium/src/ui/gfx/color_palette.h?type=cs&q=kGoogleRed600&g=0&l=25
Do you want the CL for this merged back into M-68?
bettes@ - Here are the updated screenshots, based on https://chromium-review.googlesource.com/1118609.

In addition to fixing the color of the error text, I've also:
- Removed the X (Close) button (we were removing all the others, and I couldn't see any reason to keep this one; let me know if you want me to put it back)
- Fixed the width to ACTUALLY be 448px (I added code for this the first time around, but some other unnoticed code was "fixing" it to 512px)

Please take a look, and let me know if this version is okay.


hd_pinreq_valid_v2.png
17.4 KB View Download
hd_pinreq_invalid_v2.png
20.9 KB View Download
Thanks so much, xdai@. Latest screenshots LGTM!
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 13

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

commit 243c4f4191239b77159dba472b6819aa2d0c1850
Author: glevin <glevin@chromium.org>
Date: Fri Jul 13 17:31:21 2018

Harmonize RequestPinView dialog - Part II

As per  crbug.com/788024  Comment 10 and other observations:
- Make error text darker (#D93025)
- Dialog width = 448 (for reals this time)
- Remove X (Close) button from upper right

Bug:  788024 
Test: Get yourself into some kinda pin-needin' situation and confirm the
Harmonious nature of the pin-requestin' dialog.

Change-Id: I7da3bffa3ff91434f651dd316a245cdf8d3f91a1
Reviewed-on: https://chromium-review.googlesource.com/1118609
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Greg Levin <glevin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574961}
[modify] https://crrev.com/243c4f4191239b77159dba472b6819aa2d0c1850/chrome/browser/chromeos/ui/request_pin_view.cc
[modify] https://crrev.com/243c4f4191239b77159dba472b6819aa2d0c1850/chrome/browser/chromeos/ui/request_pin_view.h
[modify] https://crrev.com/243c4f4191239b77159dba472b6819aa2d0c1850/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment