Harmony - update "Reset Settings?" dialogs |
||||||||||||
Issue description"Restore homepage" and other SettingsResetPromptDialog variations: * Homepage * Search engine * Startup page Mock: https://docs.google.com/presentation/d/19cryphS69HT-8OJNv_fV-0N2I7ftMQDblle8fpO_lL0/edit?ts=58b9de3c#slide=id.g1ec868bd9b_0_14
,
Sep 5 2017
,
Sep 5 2017
,
Sep 21 2017
,
Oct 19 2017
,
Oct 19 2017
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8e15c2c1a409bd175e147ecfd6f703198bbc6b5 commit e8e15c2c1a409bd175e147ecfd6f703198bbc6b5 Author: Peter Boström <pbos@chromium.org> Date: Thu Oct 19 00:59:21 2017 Rename tests for SettingsResetPromptDialog. Expands test names like dse_ext1 for instance to something easily readable. Bug: chromium:698785 Change-Id: I96a65b377f9d882f2f9a15832bb701cfe24afe4d Reviewed-on: https://chromium-review.googlesource.com/727364 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#509952} [modify] https://crrev.com/e8e15c2c1a409bd175e147ecfd6f703198bbc6b5/chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc
,
Nov 10 2017
The NextAction date has arrived: 2017-11-10
,
Dec 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e2316d2ddb903885176ddca4f981cd6e0389de8 commit 4e2316d2ddb903885176ddca4f981cd6e0389de8 Author: Peter Boström <pbos@chromium.org> Date: Wed Dec 13 03:20:41 2017 Add use_titlecase entries for SettingsResetPrompt. Copies the strings and string descriptions verbatim but makes the use_titlecase entry In Title Case. The Restore label is the same in both but an additional copy is made to make sure that other languages could title case it if appropriate. Bug: chromium:698785 Change-Id: I923457767c155cdf4c147402a0ca7bc8376319fd Reviewed-on: https://chromium-review.googlesource.com/822136 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#523673} [modify] https://crrev.com/4e2316d2ddb903885176ddca4f981cd6e0389de8/chrome/app/generated_resources.grd
,
Dec 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a63b76746707a30e7c4bad9a1cdb005a14ee571e commit a63b76746707a30e7c4bad9a1cdb005a14ee571e Author: Peter Boström <pbos@chromium.org> Date: Wed Dec 13 20:12:51 2017 Update SettingsResetPromptDialog for Harmony. Incorporates a couple of changes: * Removes close button for this modal dialog. * Account for margins in size calculation to make the dialog 448px wide. * Set label text context to CONTEXT_BODY_TEXT_LARGE. This label currently does not bolden the URL under Harmony as that's controlled by STYLE_EMPHASIZED which has no styling in Harmony. This is tracked in chromium:794325. Bug: chromium:698785 Change-Id: Iba79a5bd986bd8a2d0839ffc5145ece8647a9367 Reviewed-on: https://chromium-review.googlesource.com/823132 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#523858} [modify] https://crrev.com/a63b76746707a30e7c4bad9a1cdb005a14ee571e/chrome/browser/ui/views/settings_reset_prompt_dialog.cc [modify] https://crrev.com/a63b76746707a30e7c4bad9a1cdb005a14ee571e/chrome/browser/ui/views/settings_reset_prompt_dialog.h
,
Dec 13 2017
,
Dec 14 2017
Alan can you take a look at if there's anything left besides STYLE_EMPHASIZED highlighting the URL (blocking bug). Here are two example pages. The strings used are the ones prefixed by IDS_SETTINGS_RESET_PROMPT_ here: https://chromium.googlesource.com/chromium/src/+/4e2316d2ddb903885176ddca4f981cd6e0389de8/chrome/app/generated_resources.grd#5940
,
Dec 14 2017
,
Dec 20 2017
LGTM!
,
Dec 20 2017
Sweet!
,
Dec 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/497efd752862c762cccfe472e8e47f35c7d47933 commit 497efd752862c762cccfe472e8e47f35c7d47933 Author: Peter Boström <pbos@chromium.org> Date: Sat Dec 23 00:56:22 2017 Use bold for STYLE_EMPHASIZED in body contexts. Bug: chromium:698785 , chromium:794325 Change-Id: Idc35cd777c5d9c5232cd728cc57df9c21101f611 Reviewed-on: https://chromium-review.googlesource.com/843017 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#526115} [modify] https://crrev.com/497efd752862c762cccfe472e8e47f35c7d47933/chrome/browser/ui/views/harmony/harmony_typography_provider.cc
,
Mar 9 2018
Actually screenshot looks alright, this is body1 the old screenshot was at >100% HDPI. It got a bold update after the screenshot though per separate discussion.
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38b5c1844b450a43facea2da970facb4b02b1469 commit 38b5c1844b450a43facea2da970facb4b02b1469 Author: Peter Boström <pbos@chromium.org> Date: Thu Mar 22 18:47:21 2018 Add typography style STYLE_EMPHASIZED_SECONDARY. Uses this + STYLE_SECONDARY for the SettingsResetPromptDialog to match the desired secondary style. Without STYLE_EMPHASIZED_SECONDARY the emphasized text would be bold black which sticks out way too much against the secondary gray. Bug: chromium:698785 Change-Id: I3f9d1d3423ff3e990cc67f131f33ff0c7ddf55ff Reviewed-on: https://chromium-review.googlesource.com/974065 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#545172} [modify] https://crrev.com/38b5c1844b450a43facea2da970facb4b02b1469/chrome/browser/ui/views/harmony/chrome_typography.cc [modify] https://crrev.com/38b5c1844b450a43facea2da970facb4b02b1469/chrome/browser/ui/views/harmony/chrome_typography.h [modify] https://crrev.com/38b5c1844b450a43facea2da970facb4b02b1469/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [modify] https://crrev.com/38b5c1844b450a43facea2da970facb4b02b1469/chrome/browser/ui/views/settings_reset_prompt_dialog.cc
,
Mar 22 2018
Sorry I missed the "secondary" part of that. Attaching implementation screenshot which also includes a secondary emphasis style. Another method of styling secondary emphasis would be to use primary text for the emphasis as it'll be more prominent than the secondary text surrounding it. This dialog is very uncommon, I don't feel like a merge to M66 is warranted. Let me know if you disagree (+cc markchang@ as well).
,
Mar 23 2018
@pbos: Could you please provide the sample steps of the issue which would help us to verify the issue further. Thanks in Advance.
,
Mar 23 2018
I don't think we need a merge. |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by pkasting@chromium.org
, Aug 2 2017