MacViews: get http auth dialog ready to go |
|||||||||||||||||||||||||||||||
Issue description
,
Apr 14 2016
,
Apr 14 2016
,
Apr 14 2016
bulk-tagging Phase2 for M-53
,
Apr 14 2016
,
Apr 20 2016
,
May 9 2016
Current status: this is behind --enable-features=MacViewsWebUIDialogs.
,
Sep 23 2016
Should we re-open this? Review notes: https://groups.google.com/a/google.com/forum/#!topic/chrome-harmony/xqTrAqnJ-JE
,
Oct 8 2016
Spec is here: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec/Phase%202#%2FSPEC-Phase%202-HTTP%20auto.png%3Fz=width
,
Oct 11 2016
Initial comments: * Buttons should be 8pt apart * Buttons should be 16pt from bottom and right edge of dialog * Controls should be 16pt from left edge of dialog * Field titles should be left-justified, no colons * Input fields right edge should be 16pt from right edge of dialog * Input fields should be 288pt wide * About 22pt from bottom of input field stroke to top of buttons * Dialog should be 450pt x 244pt * Connection is not private text should not end in period * http://httpbin.org text should not be followed by "requires a username..." * Dialog text should be black (for some reason it is not black)
,
Oct 11 2016
* Sorry, dialog width should be 448pt
,
Nov 8 2016
Working on layout. before, before-shaded = before any changes, and the shaded one shows the actual bounds of various views; after, after-shaded = after prototype changes. Still known wrong: 1) Size of the entire dialog 2) Padding on the title is a bit too big (same problem as 1) 3) Button sizes aren't snapped to 16pt or 32pt increments
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/964f7ce4df1d586722cf3819eac9cc814cd98e1d commit 964f7ce4df1d586722cf3819eac9cc814cd98e1d Author: ellyjones <ellyjones@chromium.org> Date: Wed Nov 16 20:30:01 2016 views: add layout delegates This change: 1) Adds the concept of a "layout delegate" for a View, which is responsible for computing padding amounts and other minutiae of control positioning; 2) Adds a HarmonyLayoutDelegate which returns the Harmony specified values for some of the existing padding constants; 3) Changes GridLayout and DialogClientView to honor LayoutDelegate; 4) Makes chrome's LoginView use HarmonyLayoutDelegate when --secondary-ui-md. BUG= 602392 , 635173 Review-Url: https://codereview.chromium.org/2485083003 Cr-Commit-Position: refs/heads/master@{#432611} [modify] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/BUILD.gn [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/harmony/harmony_layout_delegate.h [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/harmony/layout_delegate.cc [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/harmony/layout_delegate.h [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/layout_utils.cc [add] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/layout_utils.h [modify] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/chrome/browser/ui/views/login_view.cc [modify] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/ui/views/views_delegate.cc [modify] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/ui/views/views_delegate.h [modify] https://crrev.com/964f7ce4df1d586722cf3819eac9cc814cd98e1d/ui/views/window/dialog_client_view.cc
,
Jan 25 2017
,
Jun 27 2017
Issue 600416 is polish - removing blocker. The dialog is wider now. Is there anything left? I think we should get a review stamp and close out this bug. To summon: http://httpbin.org/basic-auth/user/passwd
,
Jun 27 2017
I don't know how wide it's "meant" to be. Anyway, shrike@, what do you think of this dialog?
,
Jun 27 2017
The width is correct, but overall the dialog is still pretty far from the spec: * Please remove the colons from the field labels * There should be no close button * There is too much space between the dialog title and the first line of text (they should be 10pt closer) * There is too much space between the first line of text and the second (they should be 6pt closer) * The first line of text should show just the site URL, not the trailing " requires a username and password" * The second line of text should say, "Your connection to this site is not private" * The second line of text should be a lighter color gray * The input textfields should start 144pt from the left edge of the dialog * The spec calls for textfields that are 28pt tall with 12pt of space between them
,
Jun 28 2017
,
Jun 28 2017
yikes. Seems there is a new mock? #c11 is a dead link. Updated the description with the latest png. I hope we are not planning to update strings on all 60 dialogs at once... I'll take this. I'm keen to get progress on a simpler dialog to get some kind of spec lock-in somewhere. And I'm curious to see how the typography stuff goes on "pre-harmony" dialogs of we just ignoring whether the --secondary-ui-md flag is present. Feels too much like we are chasing tails right now.
,
Jul 5 2017
Done I guess -> https://chromium-review.googlesource.com/c/559205 There are a couple of layout constants that aren't in the LayoutProvider yet. I'm not sure we want them to be if they are specific to textfield stacks. Since, I added some helper methods to layout a textfield stack, if we expose the textfield stack layout constants then it will tempt dialog writers to implement their own textfield stack layout rather than modify the helpers to be more generic. I also haven't really gone to lengths to maintain the strings in the old dialogs. The new strings seem fine in the old layout (see attached).
,
Jul 13 2017
Talked with Peter over hangouts - revising the mock to simplify things a bit: Start the text-input 16px after the layout unit of the longest string width.
,
Jul 13 2017
,
Jul 13 2017
Updated the mock in the description. Also, since we're changing strings anyway... what strings do we want for webproxy auth requests? The current dialog says "The proxy http://example.com requires a username and password." "Your connection to this site is not private" (note there is Issue 620756 for saying something other than "Your connection to this site is not private" for proxies). https://chromium-review.googlesource.com/c/559205 drops the "requires a username and password" bit for regular requests, but doesn't touch strings for proxies.
,
Jul 13 2017
,
Jul 13 2017
screengrabs after applying latest review comments. From https://chromium-review.googlesource.com/c/559205 patchset 13.
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7029190024cf7d1125eebcd4fe65c681483c54f7 commit 7029190024cf7d1125eebcd4fe65c681483c54f7 Author: Trent Apted <tapted@chromium.org> Date: Mon Jul 17 09:25:28 2017 Update HTTP-Auth to latest Harmony mocks. With *and without* --secondary-ui-md: * Removes the colons from the field labels * Alters the text for insecure domains to say "Your connection to this site is not private" * Makes the dialog title and "Log in" button text sentence case on non-Mac/iOS * Removes " requires a username and password" after the domain for non-Proxy auth requests * Removes the insets on the sides of the textfield stack relative to the dialog title. * Removes the close button from the http-auth dialog With --secondary-ui-md only: * Uses SECONDARY / gray text for the second line of header text * Makes textfields 28px tall 3 general adjustments applicable to a class of dialogs remain to be done: * Too much spacing from the title to the body text, and * Too little spacing from the last textfield to the button row. * Textfields should horizontally align "somehow" we don't know how yet Adds ui/views/harmony/layout_helper.h to encapsulate methods for creating a textfield stack in the standard layout. Bug: 602392 Change-Id: I27fbc52c95d9bd8ac2a3551d916d3f161f51593f Reviewed-on: https://chromium-review.googlesource.com/559205 Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#487030} [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/login/login_handler.cc [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/login/login_handler_unittest.cc [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/harmony/chrome_layout_provider.cc [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/harmony/chrome_layout_provider.h [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/harmony/harmony_layout_provider.cc [add] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/harmony/textfield_layout.cc [add] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/harmony/textfield_layout.h [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/login_handler_views.cc [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/login_view.cc [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/chrome/browser/ui/views/login_view.h [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/components/login_dialog_strings.grdp [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/components/page_info_strings.grdp [modify] https://crrev.com/7029190024cf7d1125eebcd4fe65c681483c54f7/ui/views/controls/button/md_text_button.cc
,
Aug 9 2017
,
Sep 5 2017
,
Sep 5 2017
,
Oct 3 2017
Tapted, I don't believe I see the expected hairline stroke on the focused text field? LMK
,
Oct 3 2017
Here's an updated screenshot. You can also visit https://httpbin.org/basic-auth/user/passwd with chrome://flags/#secondary-ui-md enabled
,
Oct 10 2017
For https sites requiring auth, we should close the spacing between the first text input field and the url. For http sites requiring auth, the implementation LGTM.
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/974080cc9e7a2e8128c1b2353095a80d148e9ec0 commit 974080cc9e7a2e8128c1b2353095a80d148e9ec0 Author: Trent Apted <tapted@chromium.org> Date: Thu Oct 12 23:45:33 2017 Omit the empty space on http-auth when "Your connection to this site is not secure" is not displayed Per http://crbug.com/602392#c35 Bug: 602392 Change-Id: Ifaa129afa92bc8b58166e93cc8354d76e8e11e49 Reviewed-on: https://chromium-review.googlesource.com/715039 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#508544} [modify] https://crrev.com/974080cc9e7a2e8128c1b2353095a80d148e9ec0/chrome/browser/ui/views/login_view.cc
,
Oct 30 2017
c35 has been addressed and fixed. We can mark as Fixed.
,
Oct 30 2017
\o/
,
Nov 10 2017
The NextAction date has arrived: 2017-11-10 |
|||||||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||||||
/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec/Phase%202#%2FSPEC-Phase%202-HTTP%20auto.png%3Fz=width){kind=link}
Comment 1 by tapted@chromium.org
, Apr 12 2016