Harmony - update EchoDialogView |
||||||
Issue descriptionChrome Version: (copy from chrome://version) OS: Chrome See https://folio.googleplex.com/_/preview/1sFARH7Ox4zR2zb8Y6PsgWvl0Nrbxouj6/EchoDialogView#%2FScreenshot%202017-11-08%20at%2015.55.06.png%3Fz=width&c=show for screenshot and required changes.
,
Nov 23 2017
,
Nov 23 2017
,
May 2 2018
The first patchset at https://chromium-review.googlesource.com/c/chromium/src/+/1040868 generates the attached dialogs (using --enable-features=SecondaryUiMd). They seem mostly Harmonious, but the spacing between top and text seems a little thin. If more space is needed, please let me know how much.
,
May 14 2018
Yes, the spacing looks thin. We should have equal 16px padding around top, right, bottom, and left.
,
May 15 2018
Here are updated screenshots, with 16px spacing between dlg borders and content on all sides. Let me know how these look.
,
May 21 2018
LGTM
,
May 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00bba98875c4027a82579bc5bc61f79b942af969 commit 00bba98875c4027a82579bc5bc61f79b942af969 Author: glevin <glevin@chromium.org> Date: Thu May 24 04:14:31 2018 Harmonize EchoDialogView As per crbug.com/788015 Comments 1,5: - Fix padding sizes - Change button text "Dismiss" -> "Got it" - Make a button default (blue) - Replace "Learn more" link with (?) button - Button has a11y text and is Tab stop - Remove X (Close) button from upper right - Clean up code, use standard LayoutProvider Bug: 788015 Test: Set up a new Chromebook with Goodies offer available, witness a dialog living in Harmony Change-Id: I95058420c54a1b2b48881b95b458971ec163a8ae Reviewed-on: https://chromium-review.googlesource.com/1040868 Commit-Queue: Greg Levin <glevin@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#561384} [modify] https://crrev.com/00bba98875c4027a82579bc5bc61f79b942af969/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/00bba98875c4027a82579bc5bc61f79b942af969/chrome/browser/chromeos/ui/echo_dialog_view.cc [modify] https://crrev.com/00bba98875c4027a82579bc5bc61f79b942af969/chrome/browser/chromeos/ui/echo_dialog_view.h
,
Dec 24
Don't remember why this was open, but it seems done. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by x...@chromium.org
, Nov 23 2017