Get Rid of settings-multidevice-page-container element |
||||
Issue descriptionBackground: Initially we introduced this element to allow the MultiDevice settings section to dynamically hide itself when there were no eligible hosts. Since then, the specs have changed so that the section no longer needs to hide itself in that case. So now the only reasons to hide the section are the cases handled by the general settings framework (flag off, guest session) or when the Chromebook is not enrolled in CryptAuth. Because we will be enrolling Chromebooks automatically when their flag is on, the only case that's not covered by the general settings framework is when a user opens the setting page between the time they go through the Setup Flow and when their Chromebook gets its response from CryptAuth. The CryptAuth response is expected to take on the order of one second or less so the remaining problem case is when the user is offline. We could add a string to clarify this case or we could simply treat it the same as the 'no eligible devices' case since it's likely to be an infrequent case anyway. Immediate Impetus: Settings has recently pushed back against this design because no other settings page has that pattern (dynamically appearing/hiding) and it is at odds with some of their existing patterns, e.g. they use an immutable registry of subpage routes. Also, a few bugs have popped up, e.g. 883893, 876905, that are likely to go away if we don't rely on the additional layer of the settings-multidevice-page-container handling the page's visibility dynamically. Plan: Get rid of the intermediate settings-multidevice-page-container layer altogether.
,
Sep 28
,
Sep 28
Issue 876905 has been merged into this issue.
,
Sep 28
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/562605f1860d0e0c997343ce04bde1511ef0efa4 commit 562605f1860d0e0c997343ce04bde1511ef0efa4 Author: Jordy Greenblatt <jordynass@chromium.org> Date: Wed Oct 03 03:15:34 2018 [CrOS MultiDevice] Remove page container infrastructure in Settings UI Settings has strongly urged us to move away from dynamically attaching and detaching the MultiDevice page from Settings UI. After this push, I looked into the costs of that and with khorimoto@ decided they were small enough to warrant the refactor given the rarity of this functionality in practice. Note that MultiDevice settings flag already controls whether to allow the page at a higher level so that does not rely on the dynamic attachment ability provided by the multidevice-page-container. An additional incentive is to prevent a few display bugs (see the bugs merged into 887784 for details) that are very likely to be irrelevant once this infrastructure is removed. Screenshots from the cases that cause the MultiDevice page to detach in the HEAD version: http://screen/BW2npVh2Ocw http://screen/pKLechGvcCp Bug: 887784 Change-Id: Iaefb8bff4ae300c483645e2558b8a95083e2bc47 Reviewed-on: https://chromium-review.googlesource.com/c/1258211 Commit-Queue: Jordy Greenblatt <jordynass@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#596107} [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/basic_page/basic_page.html [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/basic_page/basic_page.js [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/multidevice_page/BUILD.gn [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/multidevice_page/multidevice_page.html [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/multidevice_page/multidevice_page.js [delete] https://crrev.com/762df375cb5cde323ba6681a38775778939dbddd/chrome/browser/resources/settings/multidevice_page/multidevice_page_container.html [delete] https://crrev.com/762df375cb5cde323ba6681a38775778939dbddd/chrome/browser/resources/settings/multidevice_page/multidevice_page_container.js [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/browser/resources/settings/settings_resources.grd [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/test/data/webui/settings/cr_settings_browsertest.js [delete] https://crrev.com/762df375cb5cde323ba6681a38775778939dbddd/chrome/test/data/webui/settings/multidevice_page_container_tests.js [modify] https://crrev.com/562605f1860d0e0c997343ce04bde1511ef0efa4/chrome/test/data/webui/settings/multidevice_page_tests.js
,
Oct 3
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jordynass@chromium.org
, Sep 21Status: Started (was: Untriaged)