Settings WebUI: Show sidenav menu by default on larger screens

Bug: 628247 Change-Id: Ic57069ecdb1a68bfad03f7727f2adaf69d7e42fc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1583120 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: John Lee <johntlee@chromium.org> Cr-Commit-Position: refs/heads/master@{#658706}
Denger-Network · May 10, 2019 · e69d126 · e69d126
1 parent 4f6f29a
commit e69d126
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 29 deletions.
diff --git a/chrome/browser/resources/settings/settings_menu/settings_menu.html b/chrome/browser/resources/settings/settings_menu/settings_menu.html
@@ -14,8 +14,9 @@
   <template>
     <style include="settings-shared">
       :host {
+        box-sizing: border-box;
         display: block;
-        margin-top: 8px;
+        padding-top: 8px;
       }
 
       a[href],

diff --git a/chrome/browser/resources/settings/settings_page/settings_section.html b/chrome/browser/resources/settings/settings_page/settings_section.html
@@ -9,9 +9,6 @@
       :host {
         display: flex;
         flex-direction: column;
-        /* Margin so the box-shadow isn't clipped during animations. */
-        margin-inline-end: 3px;
-        margin-inline-start: 3px;
         outline: none;
         position: relative;
       }

diff --git a/chrome/browser/resources/settings/settings_ui/settings_ui.html b/chrome/browser/resources/settings/settings_ui/settings_ui.html
@@ -4,6 +4,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_drawer/cr_drawer.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_toolbar/cr_toolbar.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/html/find_shortcut_behavior.html">
 <link rel="import" href="chrome://resources/html/util.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
@@ -30,27 +31,79 @@
         @apply --layout-fit;
         display: flex;
         flex-direction: column;
-      }
-
-      cr-drawer {
-        z-index: 2;
+        --settings-menu-width: 250px;
+        /* Centered cards has a max-width of var(--cr-centered-card-max-width)
+         * and a width of a certain percentage. Therefore, to make room for the
+         * cards to be at their max width, the flex-basis of #main needs to be
+         * whatever value the percentage of would equal to the max width. */
+        --settings-main-basis: calc(var(--cr-centered-card-max-width) /
+            var(--cr-centered-card-width-percentage));
       }
 
       cr-toolbar {
         @apply --layout-center;
         min-height: 56px;
         z-index: 2;
+        --cr-toolbar-left-spacer-width: var(--settings-menu-width);
+        --cr-toolbar-center-basis: var(--settings-main-basis);
       }
 
       :host-context(html:not([dark])) cr-toolbar {
         --iron-icon-fill-color: white;
       }
 
+      #cr-container-shadow-top {
+        /* Needs to be higher than #container's z-index to appear above
+         * scrollbars. */
+        z-index: 2;
+      }
+
       #container {
+        align-items: flex-start;
+        display: flex;
         flex: 1;
         overflow: overlay;
         position: relative;
       }
+
+      #left,
+      #main,
+      #right {
+        flex: 1 1 0;
+      }
+
+      #left {
+        height: 100%;
+        position: sticky;
+        top: 0;
+      }
+
+      #left settings-menu {
+        max-height: 100%;
+        overflow: auto;
+        overscroll-behavior: contain;
+        width: var(--settings-menu-width);
+      }
+
+      #main {
+        flex-basis: var(--settings-main-basis);
+        height: 100%;
+        min-height: fit-content;
+      }
+
+      /* The breakpoint of 980px was decided on by the rounded sum of
+       * var(--settings-main-basis), var(--settings-menu-width), and
+       * var(--cr-section-padding). */
+      @media (max-width: 980px) {
+        #left,
+        #right {
+          display: none;
+        }
+
+        #main {
+          min-width: auto;
+        }
+      }
     </style>
     <settings-prefs id="prefs" prefs="{{prefs}}"></settings-prefs>
     <cr-toolbar page-name="$i18n{settings}"
@@ -61,7 +114,9 @@
         menu-label="$i18n{menuButtonLabel}"
         on-search-changed="onSearchChanged_"
         role="banner"
-        show-menu>
+        narrow="{{narrow_}}"
+        narrow-threshold="980"
+        show-menu="[[narrow_]]">
     </cr-toolbar>
     <cr-drawer id="drawer" on-close="onMenuClose_" heading="$i18n{settings}"
         align="$i18n{textdirection}">
@@ -81,6 +136,17 @@
       </div>
     </cr-drawer>
     <div id="container" class="no-outline">
+      <div id="left">
+        <settings-menu page-visibility="[[pageVisibility_]]"
+            show-android-apps="[[showAndroidApps_]]"
+            show-crostini="[[showCrostini_]]"
+            show-plugin-vm="[[showPluginVm_]]"
+            show-multidevice="[[showMultidevice_]]"
+            have-play-store-app="[[havePlayStoreApp_]]"
+            on-iron-activate="onIronActivate_"
+            advanced-opened="{{advancedOpened_}}">
+        </settings-menu>
+      </div>
       <settings-main id="main" prefs="{{prefs}}"
           toolbar-spinner-active="{{toolbarSpinnerActive_}}"
           page-visibility="[[pageVisibility_]]"
@@ -93,6 +159,9 @@
           have-play-store-app="[[havePlayStoreApp_]]"
           advanced-toggle-expanded="{{advancedOpened_}}">
       </settings-main>
+      <!-- An additional child of the flex #container to take up space,
+           aligned with the right-hand child of the flex toolbar. -->
+      <div id="right"></div>
     </div>
   </template>
   <script src="settings_ui.js"></script>

diff --git a/chrome/browser/resources/settings/settings_ui/settings_ui.js b/chrome/browser/resources/settings/settings_ui/settings_ui.js
@@ -45,6 +45,9 @@ Polymer({
       value: false,
     },
 
+    /** @private */
+    narrow_: Boolean,
+
     /**
      * @private {!PageVisibility}
      */

diff --git a/chrome/test/data/webui/settings/settings_ui_browsertest.js b/chrome/test/data/webui/settings/settings_ui_browsertest.js
@@ -48,13 +48,13 @@ TEST_F('SettingsUIBrowserTest', 'MAYBE_All', function() {
       Polymer.dom.flush();
     });
 
-    test('basic', function() {
+    test('showing menu in toolbar', function() {
       toolbar = assert(ui.$$('cr-toolbar'));
-      assertTrue(toolbar.showMenu);
+      assertFalse(toolbar.showMenu);
     });
 
     test('app drawer', function() {
-      assertEquals(null, ui.$$('settings-menu'));
+      assertEquals(null, ui.$$('cr-drawer settings-menu'));
       const drawer = ui.$.drawer;
       assertFalse(!!drawer.open);
 
@@ -64,7 +64,7 @@ TEST_F('SettingsUIBrowserTest', 'MAYBE_All', function() {
 
       // Validate that dialog is open and menu is shown so it will animate.
       assertTrue(drawer.open);
-      assertTrue(!!ui.$$('settings-menu'));
+      assertTrue(!!ui.$$('cr-drawer settings-menu'));
 
       return whenDone
           .then(function() {
@@ -76,37 +76,43 @@ TEST_F('SettingsUIBrowserTest', 'MAYBE_All', function() {
             // Drawer is closed, but menu is still stamped so
             // its contents remain visible as the drawer slides
             // out.
-            assertTrue(!!ui.$$('settings-menu'));
+            assertTrue(!!ui.$$('cr-drawer settings-menu'));
           });
     });
 
     test('advanced UIs stay in sync', function() {
       const main = ui.$$('settings-main');
+      const floatingMenu = ui.$$('#left settings-menu');
       assertTrue(!!main);
+      assertTrue(!!floatingMenu);
 
-      assertFalse(!!ui.$$('settings-menu'));
+      assertFalse(!!ui.$$('cr-drawer settings-menu'));
       assertFalse(ui.advancedOpened_);
+      assertFalse(floatingMenu.advancedOpened);
       assertFalse(main.advancedToggleExpanded);
 
       main.advancedToggleExpanded = true;
       Polymer.dom.flush();
 
-      assertFalse(!!ui.$$('settings-menu'));
+      assertFalse(!!ui.$$('cr-drawer settings-menu'));
       assertTrue(ui.advancedOpened_);
+      assertTrue(floatingMenu.advancedOpened);
       assertTrue(main.advancedToggleExpanded);
 
       ui.$.drawerTemplate.if = true;
       Polymer.dom.flush();
 
-      const menu = ui.$$('settings-menu');
-      assertTrue(!!menu);
-      assertTrue(menu.advancedOpened);
+      const drawerMenu = ui.$$('cr-drawer settings-menu');
+      assertTrue(!!drawerMenu);
+      assertTrue(floatingMenu.advancedOpened);
+      assertTrue(drawerMenu.advancedOpened);
 
-      menu.$.advancedButton.click();
+      drawerMenu.$.advancedButton.click();
       Polymer.dom.flush();
 
       // Check that all values are updated in unison.
-      assertFalse(menu.advancedOpened);
+      assertFalse(drawerMenu.advancedOpened);
+      assertFalse(floatingMenu.advancedOpened);
       assertFalse(ui.advancedOpened_);
       assertFalse(main.advancedToggleExpanded);
     });

diff --git a/ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html b/ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html
@@ -42,16 +42,17 @@
       }
 
       #leftContent {
-        /* margin-start here must match margin-end on #rightContent. */
-        margin-inline-start: 12px;
         position: relative;
         transition: opacity 100ms;
       }
 
       #leftSpacer {
         align-items: center;
+        box-sizing: border-box;
         display: flex;
-        margin-inline-start: 6px;
+        /* 12px to match #rightSpacer + 6px to align with icons in menus. */
+        padding-inline-start: calc(12px + 6px);
+        width: var(--cr-toolbar-left-spacer-width, auto);
       }
 
       cr-icon-button {
@@ -69,8 +70,8 @@
         justify-content: center;
       }
 
-      #rightContent {
-        margin-inline-end: 12px;
+      #rightSpacer {
+        padding-inline-end: 12px;
       }
 
       :host([narrow]) #centeredContent {
@@ -91,6 +92,10 @@
         flex: 1 1 var(--cr-toolbar-field-margin, 0);
       }
 
+      :host(:not([narrow])) #centeredContent {
+        flex-basis: var(--cr-toolbar-center-basis, 0);
+      }
+
       :host(:not([narrow])) #rightContent {
         flex: 1 1 0;
         text-align: end;
@@ -178,7 +183,9 @@ <h1>[[pageName]]</h1>
     </div>
 
     <div id="rightContent">
-      <slot></slot>
+      <div id="rightSpacer">
+        <slot></slot>
+      </div>
     </div>
   </template>
   <script src="cr_toolbar.js"></script>

diff --git a/ui/webui/resources/cr_elements/shared_vars_css.html b/ui/webui/resources/cr_elements/shared_vars_css.html
@@ -186,15 +186,16 @@
     --cr-section-vertical-margin: 21px;
 
     --cr-centered-card-max-width: 680px;
+    --cr-centered-card-width-percentage: 0.96;
     --cr-centered-card-container: {
       box-sizing: border-box;
       display: block;
       height: inherit;
       margin: 0 auto;
-      max-width: calc(var(--cr-centered-card-max-width) + 3 * 2px);
+      max-width: var(--cr-centered-card-max-width);
       min-width: 550px;
       position: relative;
-      width: 96%;
+      width: calc(100% * var(--cr-centered-card-width-percentage));
     }
 
     --cr-card-external-title: {