Add checkbox for "Control how this works in Settings" when MD settings is enabled |
|||||
Issue descriptionPer offline discussion, when MD settings is enabled, we should use a checkbox for "Control how this works in Settings" instead of link-ifying [Settings]. This will make it more clear for users that want to go configure sync settings in the advanced sync settings subpage that after they leave that subpage, sync will start.
,
Apr 6 2017
screenshots of in-progress implementation
,
Apr 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cafbdba2f02dddaa020d228b591051692bcae535 commit cafbdba2f02dddaa020d228b591051692bcae535 Author: dbeam <dbeam@chromium.org> Date: Thu Apr 06 23:22:26 2017 Sync confirmation: change "Settings" link to checkbox This is so all users must confirm sign in via "OK, GOT IT" instead of clicking "Settings" link (which closes the dialog without either of the "UNDO" or "OK, GOT IT" buttons being clicked). R=rogerta@chromium.org BUG= 709113 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2800683008 Cr-Commit-Position: refs/heads/master@{#462675} [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/app/generated_resources.grd [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/resources/signin/sync_confirmation/sync_confirmation.html [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/resources/signin/sync_confirmation/sync_confirmation.js [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/sync/one_click_signin_sync_starter.cc [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/sync/one_click_signin_sync_starter.h [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/webui/signin/sync_confirmation_handler.h [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc [modify] https://crrev.com/cafbdba2f02dddaa020d228b591051692bcae535/chrome/browser/ui/webui/signin/sync_confirmation_ui.cc
,
Apr 7 2017
,
Apr 7 2017
Let's make sure that the checkbox text is aligned with the other text and of the same color, weight, and size of the secondary body text of this dialog. Thanks!
,
Apr 7 2017
Also, just wanted to double check: we decided that this checkbox should only be shown if MD settings is enabled, right?
,
Apr 7 2017
that's what we decided but i accidentally just made it so for all cases. if we launch MD settings in M59, i'll leave everything as is. if we don't, I'll revert this specific CL on the M58 branch.
,
Apr 7 2017
correction: I'll revert the CL on the **59** branch
,
Apr 7 2017
#8-9 SGTM One more nit: I noticed on Mac that the checkbox seems to temporarily get focus as the animation is finishing. Attaching a gif for reference.
,
Apr 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fbe458bd546647113889eb95afdba9cce3d8b236 commit fbe458bd546647113889eb95afdba9cce3d8b236 Author: dbeam <dbeam@chromium.org> Date: Sat Apr 08 00:19:58 2017 Sync confirmation: tweak "Control how this works in settings" checkbox spacing R=tommycli@chromium.org BUG= 709113 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2803373002 Cr-Commit-Position: refs/heads/master@{#463068} [modify] https://crrev.com/fbe458bd546647113889eb95afdba9cce3d8b236/chrome/browser/resources/signin/sync_confirmation/sync_confirmation.html
,
Apr 8 2017
please file a separate bug about changing the string if we find something better
,
Apr 8 2017
,
Apr 9 2017
Do we think that the extraneous focus issue on Mac mentioned in #10 is minor enough to leave be for now? Would you like me to file a separate bug for it instead? |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ew...@chromium.org
, Apr 6 2017