[NTP] Clean-up GridLayoutForNtpShortcuts and remove old reorder flow

Bug: 851335
Change-Id: Ib73f8952c5e0e01f8a2f9523bfec65d5cf34b55e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854418
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Kyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705755}

chrome/browser/resources/local_ntp/externs.js

@@ -27,7 +27,6 @@ let MostVisitedData;
  * chrome/browser/search/local_ntp_source.cc:
  *     LocalNtpSource::SearchConfigurationProvider::UpdateConfigData()
  * @typedef {{chromeColors: boolean,
- *            enableShortcutsGrid: boolean,
  *            googleBaseUrl: string,
  *            isAccessibleBrowser: boolean,
  *            isGooglePage: boolean,

chrome/browser/resources/local_ntp/local_ntp.js

@@ -370,9 +370,6 @@ function createIframes() {
 
   if (configData.isGooglePage) {
     args.push('enableCustomLinks=1');
-    if (configData.enableShortcutsGrid) {
-      args.push('enableGrid=1');
-    }
     args.push(
         'addLink=' +
         encodeURIComponent(configData.translatedStrings.addLinkTitle));

chrome/browser/resources/local_ntp/most_visited_single.css

@@ -54,31 +54,13 @@ a:visited {
 
 #mv-tiles,
 .mv-tiles-old {
-  display: flex;
-  flex-wrap: wrap;
   font-size: 0;
-  justify-content: center;
-  /* 5 112px tiles per row. If you change this, also change the corresponding
-   * values in local_ntp.css. */
-  max-width: calc(var(--md-tile-size) * var(--md-max-tiles-row));
   opacity: 0;
-  position: static;
-  /* This align correctly for both LTR and RTL */
-  text-align: -webkit-auto;
+  position: relative;
   transition: opacity 300ms;
   user-select: none;
 }
 
-body.grid-layout #mv-tiles,
-body.grid-layout .mv-tiles-old {
-  display: block;
-  flex-wrap: unset;
-  justify-content: unset;
-  max-width: unset;
-  position: relative;
-  text-align: unset;
-}
-
 .mv-tiles-old {
   left: 0;
   margin: auto;
@@ -101,11 +83,11 @@ body.grid-layout .mv-tiles-old {
 
 /* Prevent transitions on the held tile in order for it to smoothly follow the
  * mouse. */
-.grid-reorder .grid-tile {
+.reorder .grid-tile {
   transition-duration: 0s;
 }
 
-.grid-reorder {
+.reorder {
   z-index: 10;  /* Ensure the held tile is visible. */
 }
 
@@ -124,46 +106,24 @@ body.grid-layout .mv-tiles-old {
   width: var(--md-tile-size);
 }
 
-.reorder {
-  background-color: white;
-  border-radius: 4px;
-  box-shadow: 0 1px 3px 0 rgba(var(--GG800-rgb), .3),
-      0 4px 8px 3px rgba(var(--GG800-rgb), .15);
-  color: rgb(var(--GG800-rgb));
-  transition-duration: 200ms;
-}
-
-@media (prefers-color-scheme: dark) {
-  .reorder {
-    background-color: rgb(var(--dark-mode-bg-rgb));
-    box-shadow: 0 1px 3px 0 rgba(0, 0, 0, .4),
-        0 4px 8px 3px rgba(0, 0, 0, .25);
-    color: rgb(var(--GG100-rgb));
-  }
-}
-
-.reorder .md-tile-inner {
-  z-index: unset;
-}
-
 .md-empty-tile {
   display: none;
 }
 
 body:not(.reordering) .md-tile:hover,
-.grid-reorder .md-tile {
+.reorder .md-tile {
   background-color: rgba(var(--GG900-rgb), .06);
 }
 
 @media (prefers-color-scheme: dark) {
   body:not(.reordering) .md-tile:hover,
-  .grid-reorder .md-tile {
+  .reorder .md-tile {
     background-color: rgba(255, 255, 255, .1);
   }
 }
 
 body.dark-theme:not(.reordering) .md-tile:hover,
-body.dark-theme .grid-reorder .md-tile {
+body.dark-theme .reorder .md-tile {
   background-color: rgba(255, 255, 255, .1);
 }
 
@@ -254,7 +214,7 @@ body.mac-chromeos .md-title {
 }
 
 /* Apply when a custom background is set. */
-body.custom-background .md-tile:not(.reorder) .md-title {
+body.custom-background .md-title {
   filter: drop-shadow(0 0 6px rgba(0, 0, 0, .35));
 }
 
@@ -263,12 +223,9 @@ body.use-title-container .md-title {
   background-color: white;
   /* Necessary for a "pill" shape. Using 50% creates an oval. */
   border-radius: 500px;
-  padding: 0 8px;
-}
-
-body.use-title-container .md-tile:not(.reorder) {
   color: rgb(var(--GG800-rgb));
   filter: unset;
+  padding: 0 8px;
 }
 
 .md-menu {
@@ -343,13 +300,13 @@ body:not(.reordering) .md-menu:focus::after {
   }
 }
 
-body.dark-theme .md-tile:not(.reorder) .md-menu::after,
+body.dark-theme .md-tile .md-menu::after,
 body.dark-theme:not(.reordering) .md-menu:focus:not(.mouse-navigation)::after {
   background-color: white;
 }
 
 @media (prefers-color-scheme: dark) {
-  body.dark-theme .md-tile:not(.reorder) .md-menu::after,
+  body.dark-theme .md-tile .md-menu::after,
   body.dark-theme:not(.reordering)
       .md-menu:focus:not(.mouse-navigation)::after {
     background-color: rgb(var(--GG200-rgb));

chrome/browser/resources/local_ntp/most_visited_single.js

@@ -46,9 +46,6 @@ const IDS = {
  */
 const CLASSES = {
   FAILED_FAVICON: 'failed-favicon',  // Applied when the favicon fails to load.
-  GRID_LAYOUT: 'grid-layout',
-  // Applied to the grid tile being moved while reordering.
-  GRID_REORDER: 'grid-reorder',
   GRID_TILE: 'grid-tile',
   GRID_TILE_CONTAINER: 'grid-tile-container',
   REORDER: 'reorder',  // Applied to the tile being moved while reordering.
@@ -170,19 +167,6 @@ let tiles = null;
  */
 let queryArgs = {};
 
-/**
- * True if we are currently reordering the tiles.
- * @type {boolean}
- */
-let reordering = false;
-
-/**
- * The tile that is being moved during the reorder flow. Null if we are
- * currently not reordering.
- * @type {?Element}
- */
-let elementToReorder = null;
-
 /**
  * True if the custom links feature is enabled, i.e. when this is a Google NTP.
  * Set when the iframe is initialized.
@@ -190,26 +174,12 @@ let elementToReorder = null;
  */
 let customLinksFeatureEnabled = false;
 
-/**
- * True if the grid layout is enabled.
- * @type {boolean}
- */
-let isGridEnabled = false;
-
 /**
  * The current grid of tiles.
  * @type {?Grid}
  */
 let currGrid = null;
 
-/**
- * Called by tests to enable the grid layout.
- */
-function enableGridLayoutForTesting() {
-  isGridEnabled = true;
-  document.body.classList.add(CLASSES.GRID_LAYOUT);
-}
-
 /**
  * Additional API for Array. Moves the item at index |from| to index |to|.
  * @param {number} from Index of the item to move.
@@ -546,7 +516,7 @@ class Grid {
     this.newIndexOfItemToReorder_ = index;
 
     // Apply reorder styling.
-    tile.classList.add(CLASSES.GRID_REORDER);
+    tile.classList.add(CLASSES.REORDER);
     // Disable other hover/active styling for all tiles.
     document.body.classList.add(CLASSES.REORDERING);
 
@@ -598,7 +568,7 @@ class Grid {
     const index = Number(tile.getAttribute('index'));
 
     // Remove reorder styling.
-    tile.classList.remove(CLASSES.GRID_REORDER);
+    tile.classList.remove(CLASSES.REORDER);
     document.body.classList.remove(CLASSES.REORDERING);
 
     // Move the tile to its new position and notify EmbeddedSearchAPI that the
@@ -623,7 +593,7 @@ class Grid {
   reorderToIndexAtPoint_(x, y) {
     const elements = document.elementsFromPoint(x, y);
     for (let i = 0; i < elements.length; i++) {
-      if (elements[i].classList.contains('grid-tile-container') &&
+      if (elements[i].classList.contains(CLASSES.GRID_TILE_CONTAINER) &&
           elements[i].getAttribute('index') !== null) {
         this.reorderToIndex_(Number(elements[i].getAttribute('index')));
         return;
@@ -936,18 +906,9 @@ function swapInNewTiles() {
   cur.id = IDS.MV_TILES;
   parent.appendChild(cur);
 
-  if (isGridEnabled) {
-    // Initialize the new tileset before modifying opacity. This will prevent
-    // the transform transition from applying after the tiles fade in.
-    currGrid.init(cur);
-  } else {
-    // Re-balance the tiles if there are more than |MD_MAX_TILES_PER_ROW| in
-    // order to make even rows.
-    if (cur.childNodes.length > MD_MAX_TILES_PER_ROW) {
-      cur.style.maxWidth = 'calc(var(--md-tile-size) * ' +
-          Math.ceil(cur.childNodes.length / 2) + ')';
-    }
-  }
+  // Initialize the new tileset before modifying opacity. This will prevent the
+  // transform transition from applying after the tiles fade in.
+  currGrid.init(cur);
 
   const flushOpacity = () => window.getComputedStyle(cur).opacity;
 
@@ -1036,80 +997,6 @@ function editCustomLink(rid) {
   window.parent.postMessage({cmd: 'startEditLink', rid: rid}, DOMAIN_ORIGIN);
 }
 
-/**
- * Starts the reorder flow. Updates the visual style of the held tile to
- * indicate that it is being moved.
- * @param {!Element} tile Tile that is being moved.
- */
-function startReorder(tile) {
-  reordering = true;
-  elementToReorder = tile;
-
-  tile.classList.add(CLASSES.REORDER);
-  // Disable other hover/active styling for all tiles.
-  document.body.classList.add(CLASSES.REORDERING);
-
-  document.addEventListener('dragend', () => {
-    stopReorder(tile);
-  }, {once: true});
-}
-
-/**
- * Stops the reorder flow. Resets the held tile's visual style and tells the
- * EmbeddedSearchAPI that a tile has been moved.
- * @param {!Element} tile Tile that has been moved.
- */
-function stopReorder(tile) {
-  reordering = false;
-  elementToReorder = null;
-
-  tile.classList.remove(CLASSES.REORDER);
-  document.body.classList.remove(CLASSES.REORDERING);
-
-  // Update |data-pos| for all tiles and notify EmbeddedSearchAPI that the tile
-  // has been moved.
-  const allTiles = document.querySelectorAll('#mv-tiles .' + CLASSES.MD_TILE);
-  for (let i = 0; i < allTiles.length; i++) {
-    allTiles[i].setAttribute('data-pos', i);
-  }
-  chrome.embeddedSearch.newTabPage.reorderCustomLink(
-      Number(tile.getAttribute('data-rid')),
-      Number(tile.getAttribute('data-pos')));
-}
-
-/**
- * Sets up event listeners necessary for tile reordering.
- * @param {!Element} tile Tile on which to set the event listeners.
- */
-function setupReorder(tile) {
-  // Starts the reorder flow.
-  tile.addEventListener('dragstart', (event) => {
-    if (!reordering) {
-      startReorder(tile);
-    }
-  });
-
-  tile.addEventListener('dragover', (event) => {
-    // Only executed when the reorder flow is ongoing. Inserts the tile that is
-    // being moved before/after this |tile| according to order in the list.
-    if (reordering && elementToReorder && elementToReorder != tile) {
-      // Determine which side to insert the element on:
-      // - If the held tile comes after the current tile, insert behind the
-      //   current tile.
-      // - If the held tile comes before the current tile, insert in front of
-      //   the current tile.
-      let insertBefore;  // Element to insert the held tile behind.
-      if (tile.compareDocumentPosition(elementToReorder) &
-          Node.DOCUMENT_POSITION_FOLLOWING) {
-        insertBefore = tile;
-      } else {
-        insertBefore = tile.nextSibling;
-      }
-      $('mv-tiles').insertBefore(elementToReorder, insertBefore);
-    }
-  });
-}
-
 /**
  * Renders a MostVisited tile (i.e. shortcut) to the DOM.
  * @param {!MostVisitedData} data Object containing rid, url, title, favicon,
@@ -1273,16 +1160,7 @@ function renderTile(data) {
     mdTile.appendChild(mdMenu);
   }
 
-  if (isGridEnabled) {
-    return currGrid.createGridTile(mdTile, data.rid, !!data.isAddButton);
-  } else {
-    // Enable reordering.
-    if (isCustomLinksEnabled() && !data.isAddButton) {
-      mdTile.draggable = 'true';
-      setupReorder(mdTile);
-    }
-    return mdTile;
-  }
+  return currGrid.createGridTile(mdTile, data.rid, !!data.isAddButton);
 }
 
 /**
@@ -1318,12 +1196,6 @@ function init() {
     customLinksFeatureEnabled = true;
   }
 
-  // Enable grid layout.
-  if (queryArgs['enableGrid'] == '1') {
-    isGridEnabled = true;
-    document.body.classList.add(CLASSES.GRID_LAYOUT);
-  }
-
   currGrid = new Grid();
   // Set up layout updates on window resize. Throttled according to
   // |RESIZE_TIMEOUT_DELAY|.
@@ -1334,11 +1206,7 @@ function init() {
     }
     resizeTimeout = window.setTimeout(() => {
       resizeTimeout = null;
-      if (isGridEnabled) {
-        currGrid.onResize();
-      } else {
-        updateTileVisibility();
-      }
+      currGrid.onResize();
     }, RESIZE_TIMEOUT_DELAY);
   };
 
@@ -1355,7 +1223,6 @@ function listen() {
 return {
   Grid: Grid,  // Exposed for testing.
   init: init,  // Exposed for testing.
-  enableGridLayoutForTesting: enableGridLayoutForTesting,
   listen: listen,
 };
 }

chrome/browser/search/local_ntp_source.cc

@@ -610,9 +610,6 @@ class LocalNtpSource::SearchConfigurationProvider
                                ->IsAccessibleBrowser());
 
     if (is_google) {
-      config_data.SetBoolean(
-          "enableShortcutsGrid",
-          base::FeatureList::IsEnabled(features::kGridLayoutForNtpShortcuts));
       config_data.SetBoolean(
           "richerPicker",
           base::FeatureList::IsEnabled(features::kNtpCustomizationMenuV2));

chrome/browser/search/ntp_features.cc

@@ -20,11 +20,6 @@ const base::Feature kChromeColors{"ChromeColors",
 const base::Feature kChromeColorsCustomColorPicker{
     "ChromeColorsCustomColorPicker", base::FEATURE_DISABLED_BY_DEFAULT};
 
-// If enabled, the NTP shortcut layout will be replaced with a grid layout that
-// enables better animations.
-const base::Feature kGridLayoutForNtpShortcuts{
-    "GridLayoutForNtpShortcuts", base::FEATURE_ENABLED_BY_DEFAULT};
-
 // If enabled, the user will see the second version of the customization picker.
 const base::Feature kNtpCustomizationMenuV2{"NtpCustomizationMenuV2",
                                             base::FEATURE_DISABLED_BY_DEFAULT};

chrome/browser/search/ntp_features.h

@@ -14,7 +14,6 @@ namespace features {
 
 extern const base::Feature kChromeColors;
 extern const base::Feature kChromeColorsCustomColorPicker;
-extern const base::Feature kGridLayoutForNtpShortcuts;
 extern const base::Feature kNtpCustomizationMenuV2;
 
 // Note: only exposed for about:flags. Use IsNtpRealboxEnabled() instead.

chrome/test/data/local_ntp/most_visited_browsertest.js

@@ -25,7 +25,7 @@ test.mostVisited.MOST_VISITED = 'most-visited';
 test.mostVisited.CLASSES = {
   GRID_TILE: 'grid-tile',
   GRID_TILE_CONTAINER: 'grid-tile-container',
-  GRID_REORDER: 'grid-reorder',
+  REORDER: 'reorder',
   REORDERING: 'reordering',
 };
 
@@ -79,7 +79,6 @@ test.mostVisited.setUp = function() {
       });
 
   test.mostVisited.mostvisited = test.mostVisited.init();
-  test.mostVisited.mostvisited.enableGridLayoutForTesting();
   test.mostVisited.grid = new test.mostVisited.mostvisited.Grid();
 };
 
@@ -375,21 +374,21 @@ test.mostVisited.testReorderStart = function() {
     assertEquals(i, Number(tile.getAttribute('rid')));
     assertTrue(tile.firstChild.draggable);
 
-    assertFalse(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+    assertFalse(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
     assertFalse(
         document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 
     // Start the reorder flow.
     tile.firstChild.dispatchEvent(dragStart);
 
-    assertTrue(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+    assertTrue(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
     assertTrue(
         document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 
     // Stop the reorder flow.
     document.dispatchEvent(dragEnd);
 
-    assertFalse(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+    assertFalse(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
     assertFalse(
         document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
   }
@@ -400,8 +399,7 @@ test.mostVisited.testReorderStart = function() {
   assertFalse(addButton.firstChild.draggable);
   addButton.firstChild.dispatchEvent(dragStart);
 
-  assertFalse(
-      addButton.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+  assertFalse(addButton.classList.contains(test.mostVisited.CLASSES.REORDER));
   assertFalse(
       document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 };
@@ -436,7 +434,7 @@ test.mostVisited.testReorderStartTouch = function() {
   assertEquals('false', tile.getAttribute('add'));
   assertEquals(0, Number(tile.getAttribute('rid')));
 
-  assertFalse(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+  assertFalse(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
   assertFalse(
       document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 
@@ -444,14 +442,14 @@ test.mostVisited.testReorderStartTouch = function() {
   tile.firstChild.dispatchEvent(touchStart);
   tile.firstChild.dispatchEvent(touchMove);
 
-  assertTrue(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+  assertTrue(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
   assertTrue(
       document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 
   // Stop the reorder flow.
   tile.firstChild.dispatchEvent(touchEnd);
 
-  assertFalse(tile.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+  assertFalse(tile.classList.contains(test.mostVisited.CLASSES.REORDER));
   assertFalse(
       document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 
@@ -461,8 +459,7 @@ test.mostVisited.testReorderStartTouch = function() {
   addButton.firstChild.dispatchEvent(touchStart);
   addButton.firstChild.dispatchEvent(touchMove);
 
-  assertFalse(
-      addButton.classList.contains(test.mostVisited.CLASSES.GRID_REORDER));
+  assertFalse(addButton.classList.contains(test.mostVisited.CLASSES.REORDER));
   assertFalse(
       document.body.classList.contains(test.mostVisited.CLASSES.REORDERING));
 };