WebUI NTP: customize-dialog, allow menu and page to be independently scrollable

Bug: 1048336
Change-Id: I49b88b1e39030f0bc990934bee9076bd52b5f35a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037324
Commit-Queue: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741548}

chrome/browser/resources/new_tab_page/customize_dialog.html

@@ -3,18 +3,62 @@
     min-width: 800px;
   }
 
+  :host {
+    --border-width: 1px;
+  }
+
+  div[slot=title] {
+    align-items: center;
+    display: flex;
+    flex-direction: row;
+    height: 80px;
+    padding: 0;
+  }
+
   div[slot=body] {
     color: var(--cr-primary-text-color);
     display: flex;
     flex-direction: row;
-    min-height: 423px;
-    padding-inline-start: 0;
+    overflow: hidden;
+    padding: 0;
+  }
+
+  #menuContainer,
+  #pagesContainer {
+    overflow: hidden;
+  }
+
+  #leftTitleSpacer,
+  #menuContainer {
+    flex-basis: 232px;
+  }
+
+  #title,
+  #pagesContainer {
+    flex-grow: 1;
+  }
+
+  #menu,
+  #pages {
+    height: calc(100% - 2 * var(--border-width));
+    overflow: auto;
+  }
+
+  #pages > iron-pages {
+    min-height: 389px;
+  }
+
+  div[scroll-border] {
+    border-bottom: var(--border-width) solid var(--ntp-border-color);
+  }
+
+  div[scroll-border]:not([show]) {
+    --ntp-border-color: transparent;
   }
 
   #menu {
     display: flex;
     flex-direction: column;
-    margin-inline-end: 40px;
   }
 
   .menu-item {
@@ -24,6 +68,7 @@
     cursor: pointer;
     display: flex;
     flex-direction: row;
+    flex-shrink: 0;
     font-size: 14px;
     height: 32px;
     margin-bottom: 16px;
@@ -74,44 +119,49 @@
   #themesIcon {
     -webkit-mask-image: url(icons/colors.svg);
   }
-
-  #pages {
-    align-items: center;
-    display: flex;
-    flex-direction: column;
-    flex-grow: 1;
-  }
 </style>
 <cr-dialog id="dialog" show-on-attach>
+  <div slot="title">
+    <div id="leftTitleSpacer"></div>
+    <div id="title">$i18n{customizeThisPage}</div>
+  </div>
   <!-- TODO(crbug.com/1040256): Currently, the sidebar scrolls in sync with the
        page content area. Fix, so that the page content can scroll
        separately. -->
   <div slot="body">
-    <iron-selector id="menu" selected-attribute="selected"
-        attr-for-selected="page-name" selected="{{selectedPage_}}"
-        on-keydown="onMenuItemKeyDown_">
-      <div class="menu-item" page-name="backgrounds" tabindex="0">
-        <div id="backgroundsIcon" class="menu-item-icon"></div>
-        $i18n{backgroundsMenuItem}
-      </div>
-      <div class="menu-item" page-name="shortcuts" tabindex="0">
-        <div id="shortcutsIcon" class="menu-item-icon"></div>
-        $i18n{shortcutsMenuItem}
+    <div id="menuContainer">
+      <div id="menu">
+        <iron-selector selected-attribute="selected"
+            attr-for-selected="page-name" selected="{{selectedPage_}}"
+            on-keydown="onMenuItemKeyDown_">
+          <div class="menu-item" page-name="backgrounds" tabindex="0">
+            <div id="backgroundsIcon" class="menu-item-icon"></div>
+            $i18n{backgroundsMenuItem}
+          </div>
+          <div class="menu-item" page-name="shortcuts" tabindex="0">
+            <div id="shortcutsIcon" class="menu-item-icon"></div>
+            $i18n{shortcutsMenuItem}
+          </div>
+          <div class="menu-item" page-name="themes" tabindex="0">
+            <div id="themesIcon" class="menu-item-icon"></div>
+            $i18n{themesMenuItem}
+          </div>
+        </iron-selector>
       </div>
-      <div class="menu-item" page-name="themes" tabindex="0">
-        <div id="themesIcon" class="menu-item-icon"></div>
-        $i18n{themesMenuItem}
+    </div>
+    <div id="pagesContainer">
+      <div id="pages">
+        <iron-pages selected="[[selectedPage_]]" attr-for-selected="page-name">
+          <!-- TODO(crbug.com/1032328): Add support for selecting background
+               image. -->
+          <div page-name="backgrounds">backgrounds</div>
+          <ntp-customize-shortcuts page-name="shortcuts">
+          </ntp-customize-shortcuts>
+          <ntp-customize-themes page-name="themes" theme="[[theme]]">
+          </ntp-customize-themes>
+        </iron-pages>
       </div>
-    </iron-selector>
-    <iron-pages id="pages" selected="[[selectedPage_]]"
-        attr-for-selected="page-name">
-      <!-- TODO(crbug.com/1032328): Add support for selecting background
-           image. -->
-      <div page-name="backgrounds">backgrounds</div>
-      <ntp-customize-shortcuts page-name="shortcuts"></ntp-customize-shortcuts>
-      <ntp-customize-themes page-name="themes" theme="[[theme]]">
-      </ntp-customize-themes>
-    </iron-pages>
+    </div>
   </div>
   <div slot="button-container">
     <cr-button class="cancel-button" on-click="onCancelClick_">

chrome/browser/resources/new_tab_page/customize_dialog.js

@@ -43,6 +43,49 @@ class CustomizeDialogElement extends PolymerElement {
     super();
     /** @private {newTabPage.mojom.PageHandlerRemote} */
     this.pageHandler_ = BrowserProxy.getInstance().handler;
+    /** @private {!Array<!IntersectionObserver>} */
+    this.intersectionObservers_ = [];
+  }
+
+  /** @override */
+  disconnectedCallback() {
+    super.disconnectedCallback();
+    this.intersectionObservers_.forEach(observer => {
+      observer.disconnect();
+    });
+    this.intersectionObservers_ = [];
+  }
+
+  /** @override */
+  ready() {
+    super.ready();
+
+    ['menu', 'pages'].forEach(id => {
+      const container = this.$[id];
+      const topProbe = document.createElement('div');
+      container.prepend(topProbe);
+      const bottomProbe = document.createElement('div');
+      container.append(bottomProbe);
+      const topBorder = document.createElement('div');
+      topBorder.toggleAttribute('scroll-border', true);
+      container.parentNode.insertBefore(topBorder, container);
+      const bottomBorder = document.createElement('div');
+      bottomBorder.toggleAttribute('scroll-border', true);
+      container.parentNode.insertBefore(bottomBorder, container.nextSibling);
+      const observer = new IntersectionObserver(entries => {
+        entries.forEach(({target, intersectionRatio}) => {
+          const show = intersectionRatio === 0;
+          if (target === topProbe) {
+            topBorder.toggleAttribute('show', show);
+          } else if (target === bottomProbe) {
+            bottomBorder.toggleAttribute('show', show);
+          }
+        });
+      }, {root: container});
+      observer.observe(topProbe);
+      observer.observe(bottomProbe);
+      this.intersectionObservers_.push(observer);
+    });
   }
 
   /** @private */

chrome/test/data/webui/new_tab_page/customize_dialog_focus_test.js

@@ -11,37 +11,41 @@ suite('NewTabPageCustomizeDialogFocusTest', () => {
   /** @type {!CustomizeDialogElement} */
   let customizeDialog;
 
-  setup(async () => {
+  setup(() => {
     PolymerTest.clearBody();
 
     customizeDialog = document.createElement('ntp-customize-dialog');
     document.body.appendChild(customizeDialog);
-    await flushTasks();
+    return flushTasks();
   });
 
-  test('space selects focused menu item', async () => {
+  test('space selects focused menu item', () => {
     // Arrange.
     const menuItem = customizeDialog.shadowRoot.querySelector(
-        '.menu-item[page-name="themes"');
+        '.menu-item[page-name=themes]');
     menuItem.focus();
 
     // Act.
     keydown(menuItem, ' ');
 
     // Assert.
-    assertEquals(customizeDialog.$.menu.selected, 'themes');
+    const selector = customizeDialog.$.menu.querySelector('iron-selector');
+    assertTrue(!!selector);
+    assertEquals('themes', selector.selected);
   });
 
-  test('enter selects focused menu item', async () => {
+  test('enter selects focused menu item', () => {
     // Arrange.
     const menuItem = customizeDialog.shadowRoot.querySelector(
-        '.menu-item[page-name="shortcuts"');
+        '.menu-item[page-name=shortcuts]');
     menuItem.focus();
 
     // Act.
     keydown(menuItem, 'Enter');
 
     // Assert.
-    assertEquals(customizeDialog.$.menu.selected, 'shortcuts');
+    const selector = customizeDialog.$.menu.querySelector('iron-selector');
+    assertTrue(!!selector);
+    assertEquals('shortcuts', selector.selected);
   });
 });

chrome/test/data/webui/new_tab_page/customize_dialog_test.js

@@ -4,18 +4,18 @@
 
 import 'chrome://new-tab-page/customize_dialog.js';
 
-import {flushTasks} from 'chrome://test/test_util.m.js';
+import {flushTasks, waitAfterNextRender} from 'chrome://test/test_util.m.js';
 
 suite('NewTabPageCustomizeDialogTest', () => {
   /** @type {!CustomizeDialogElement} */
   let customizeDialog;
 
-  setup(async () => {
+  setup(() => {
     PolymerTest.clearBody();
 
     customizeDialog = document.createElement('ntp-customize-dialog');
     document.body.appendChild(customizeDialog);
-    await flushTasks();
+    return flushTasks();
   });
 
   test('creating customize dialog opens cr dialog', () => {
@@ -33,7 +33,7 @@ suite('NewTabPageCustomizeDialogTest', () => {
 
   test('selecting menu item shows page', async () => {
     // Act.
-    customizeDialog.$.menu.select('themes');
+    customizeDialog.$.menu.querySelector('[page-name=themes]').click();
     await flushTasks();
 
     // Assert.
@@ -42,4 +42,46 @@ suite('NewTabPageCustomizeDialogTest', () => {
     assertEquals(shownPages.length, 1);
     assertEquals(shownPages[0].getAttribute('page-name'), 'themes');
   });
+
+  suite('scroll borders', () => {
+    /**
+     * @param {!HTMLElement} container
+     * @private
+     */
+    async function testScrollBorders(container) {
+      const assertHidden = el => {
+        assertTrue(el.matches('[scroll-border]:not([show])'));
+      };
+      const assertShown = el => {
+        assertTrue(el.matches('[scroll-border][show]'));
+      };
+      const {firstElementChild: top, lastElementChild: bottom} = container;
+      const scrollableElement = top.nextSibling;
+      const dialogBody =
+          customizeDialog.shadowRoot.querySelector('div[slot=body]');
+      const heightWithBorders = `${scrollableElement.scrollHeight + 2}px`;
+      dialogBody.style.height = heightWithBorders;
+      assertHidden(top);
+      assertHidden(bottom);
+      dialogBody.style.height = '50px';
+      await waitAfterNextRender();
+      assertHidden(top);
+      assertShown(bottom);
+      scrollableElement.scrollTop = 1;
+      await waitAfterNextRender();
+      assertShown(top);
+      assertShown(bottom);
+      scrollableElement.scrollTop = scrollableElement.scrollHeight;
+      await waitAfterNextRender();
+      assertShown(top);
+      assertHidden(bottom);
+      dialogBody.style.height = heightWithBorders;
+      await waitAfterNextRender();
+      assertHidden(top);
+      assertHidden(bottom);
+    }
+
+    test('menu', () => testScrollBorders(customizeDialog.$.menuContainer));
+    test('pages', () => testScrollBorders(customizeDialog.$.pagesContainer));
+  });
 });