[MD settings] move OnStartup UI from sub-page to basic page |
||||||||||
Issue descriptionin chrome://settings/onStartup we'd previously moved to the OnStartup UI from the main (basic) page to a sub-page to avoid rendering/animation issues with UI changing on the main page. bettes@ requested that we revert back to the prior UI where the OnStartup UI is on the main page.
,
Sep 8 2017
Tom, also see email thread "Moving part of OnStartup back to basic page".
,
Sep 8 2017
,
Sep 8 2017
Yes, let's merge this into M62. Note that the change was made in M61, so we are acknowledging that users will see: M60: top-level M61: subpage M62: top-level (after this merge)
,
Sep 8 2017
That doesn't sound like a great user experience since we are planning to change the UI again* (likely in m63). *because having this UI at the top-level recreates issues that were being addressed by moving it to a sub-page, so we'd still need to address dynamic content changing the page while the page is opening.
,
Sep 8 2017
*because having this UI at the top-level recreates issues that were being addressed by moving it to a sub-page, so we'd still need to address dynamic content changing the page while the page is opening. Understood, but from a user's perspective, I think the benefits of having their basic controls accessible on the top-level is more important than the dynamic content issue.
,
Sep 8 2017
CL at https://chromium-review.googlesource.com/c/chromium/src/+/656258
,
Sep 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9a3169d206e066bf33f3b4e399cce5ac0782c61 commit c9a3169d206e066bf33f3b4e399cce5ac0782c61 Author: Dave Schuyler <dschuyler@chromium.org> Date: Sat Sep 09 01:32:11 2017 [MD settings] move on startup UI from sub-page to basic page This CL moves the OnStartup UI from a sub-page back to the main (basic) page. This essentially undoes (revert) CLs 557971 and 582467 while keeping the interim changes/fixes. Bug: 763186 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Idabfd1f2766c6bebaa1bc0ac72e3e7412af1ea8f Reviewed-on: https://chromium-review.googlesource.com/656258 Reviewed-by: Scott Chen <scottchen@chromium.org> Commit-Queue: Dave Schuyler <dschuyler@chromium.org> Cr-Commit-Position: refs/heads/master@{#500772} [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/app/settings_strings.grdp [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/compiled_resources2.gyp [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/on_startup_page.html [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/on_startup_page.js [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/test/data/webui/settings/on_startup_browsertest.js [modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/test/data/webui/settings/startup_urls_page_test.js
,
Sep 11 2017
,
Sep 11 2017
This bug requires manual review: There is .grd file changes and we are only 35 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 11 2017
Please add appropriate OSs.
,
Sep 11 2017
,
Sep 11 2017
candrada@, to be helpful: here are some pro / con topics for merging this CL: pro: - UI change desired by UX (bettes@) and PM (tbuckley@) - scottchen@ reviewed this CL with the knowledge that this was intended for merge (and was asked to be extra attentive to it for that reason) - This is a self contained CL and the prior code and this code are both reasonable to ship (so there's a solid fallback/rollback option here). - No features change (only moving existing features around in the UI) con: - No features change (the last to 'pro' is a 'con' depending on one's point of view).
,
Sep 11 2017
Thanks for confirming and providing more context. Can you confirm if this is already tested and verified in Canary?
,
Sep 11 2017
I can say that I see the change in Mac OSX chrome 63.0.3212.0 canary. (though it would be great to get it independently verified). Can someone from QA weigh in on "tested and verified in Canary?"
,
Sep 11 2017
Confirmed over chat that this is tested in Canary and there is a quick revert option (if needed). Approving merge to M62 (branch: 3202).
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880 commit f7fe4ab71a678e23bdbe18ef9fa01459d9efe880 Author: Dave Schuyler <dschuyler@chromium.org> Date: Mon Sep 11 23:25:38 2017 [MD settings] move on startup UI from sub-page to basic page This CL moves the OnStartup UI from a sub-page back to the main (basic) page. This essentially undoes (revert) CLs 557971 and 582467 while keeping the interim changes/fixes. Bug: 763186 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Idabfd1f2766c6bebaa1bc0ac72e3e7412af1ea8f Reviewed-on: https://chromium-review.googlesource.com/656258 Reviewed-by: Scott Chen <scottchen@chromium.org> Commit-Queue: Dave Schuyler <dschuyler@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#500772}(cherry picked from commit c9a3169d206e066bf33f3b4e399cce5ac0782c61) Reviewed-on: https://chromium-review.googlesource.com/661743 Reviewed-by: Dave Schuyler <dschuyler@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#151} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/app/settings_strings.grdp [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/compiled_resources2.gyp [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/on_startup_page.html [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/on_startup_page.js [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/test/data/webui/settings/on_startup_browsertest.js [modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/test/data/webui/settings/startup_urls_page_test.js
,
Sep 13 2017
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by dschuyler@chromium.org
, Sep 8 2017