New issue
Advanced search Search tips

Issue 774530 link

Starred by 1 user
Status: Fixed
Owner:
edinkadric@google.com
Closed: Dec 20
Cc:
vasi...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 3
Type: Bug

Blocking:
issue 753806
issue 770046



Sign in to add a comment

Save password dialog: username and password controls should be the same height (Harmony UI)

Project Member Reported by maxwalker@chromium.org, Oct 13 2017 
Chrome Version: 63.0.3239.0
OS: Mac

What is the expected result?
The username text field and password menu should both be 28dp high.

What happens instead?
The password menu is 25dp high, see attachment.
Height.png
28.3 KB View Download

Comment 1 by kolos@chromium.org, Oct 16 2017

Blocking: 753806

Comment 2 by maxwalker@chromium.org, Oct 16 2017

Labels: OS-Windows

Comment 3 by vasi...@chromium.org, Oct 16 2017 

One of the elements is a editable field and another is a combobox. For the case of one password there is no combobox and I believe that the height is the same.
I just size them to whatever the OS thinks they should be. Do you want me to hardcode a specific value for the height or maybe better just take the maximum of two?

Comment 4 by maxwalker@chromium.org, Oct 16 2017 

Changing the height of the dropdown button to match the text field sounds good to me. Maybe you could check how the Bookmark dialog is implemented?
Bookmark UI.png
24.0 KB View Download

Comment 5 by dvadym@chromium.org, Oct 19 2017

Blocking: 770046
Status: Available (was: Untriaged)

Comment 6 by benhenry@chromium.org, Aug 1

Status: Assigned (was: Available)

Comment 7 by vasi...@chromium.org, Dec 4

Cc: -battre@chromium.org -kolos@chromium.org vasi...@chromium.org
Owner: edinkadric@google.com

Comment 8 by edinkadric@google.com, Dec 6 

The bookmark dialog leaves the width and height at 0 so they end up going with an automatically-determined size. That's what we have as well, but the automatically-determined height of the password ends up being smaller, maybe because of the added eye icon that takes more space on the row.

Whatever the reason, I set it to the max of the preferred sizes in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/1365800

Attached are screenshots for old vs new for that CL (I didn't know the best way to add images there, so I'm doing it here).
Screen Shot Height Old.png
31.3 KB View Download
Screen Shot Height New.png
31.7 KB View Download

Comment 9 by edinkadric@google.com, Dec 6 

Attaching new screenshots with heights highlighted.
Screen Shot Height New.png
34.7 KB View Download
Screen Shot Height Old.png
39.0 KB View Download

Comment 10 by maxwalker@chromium.org, Dec 7 

Looks great, thanks!
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 7 

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

commit 12e333464200224e17fa040bda0b2152feb9bc06
Author: Edin Kadric <edin@google.com>
Date: Fri Dec 07 15:09:02 2018

Increase height of password combobox to match username field height.

Bug:  774530 
Change-Id: I6ce46224cf1e39a5bd6a06d100dca2a0262923e5
Reviewed-on: https://chromium-review.googlesource.com/c/1365800
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Edin Kadric <edinkadric@google.com>
Cr-Commit-Position: refs/heads/master@{#614699}
[modify] https://crrev.com/12e333464200224e17fa040bda0b2152feb9bc06/chrome/browser/ui/views/passwords/password_pending_view.cc

Comment 12 by vasi...@chromium.org, Dec 20 

Is it fixed now?

Comment 13 by edinkadric@google.com, Dec 20

Status: Fixed (was: Assigned)
Yes, I've now changed the status.
►

Sign in to add a comment