Tab Strip WebUI: Remove ghost pinned tabs

Before: https://i.imgur.com/XfCXuIb.png
After: https://i.imgur.com/Nlc9fqi.png

Bug: 989131
Change-Id: I2f8a69701fbf48fa5329f98c37e5a5bba08f82fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809802
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698611}

chrome/browser/resources/tab_strip/tab_list.html

@@ -14,23 +14,7 @@
     margin-inline-end: 16px;
   }
 
-  #pinnedTabsContainer[empty] {
-    display: none;
-  }
-
-  .ghost-pinned-tab {
-    background: var(--tabstrip-card-background-color);
-    border-radius: var(--tabstrip-card-border-radius);
-    box-shadow: var(--tabstrip-elevation-box-shadow);
-    opacity: 0.5;
-  }
-
-  /* The #pinnedTabsContainer can only fit a maximum of 4 pinned tabs. The
-   * ghost-pinned-tab elements are meant to add as placeholders if there
-   * are not enough actual pinned tabs to fill an entire column. Therefore,
-   * all ghost-pinned-tabs after the 4 * nth element should be hidden. */
-  .ghost-pinned-tab:nth-child(4n + 1),
-  .ghost-pinned-tab:nth-child(4n + 1) ~ .ghost-pinned-tab {
+  #pinnedTabsContainer:empty {
     display: none;
   }
 
@@ -43,9 +27,5 @@
   }
 </style>
 
-<div id="pinnedTabsContainer" empty>
-  <div class="ghost-pinned-tab"></div>
-  <div class="ghost-pinned-tab"></div>
-  <div class="ghost-pinned-tab"></div>
-</div>
+<div id="pinnedTabsContainer"></div>
 <div id="tabsContainer"></div>

chrome/browser/resources/tab_strip/tab_list.js

@@ -9,9 +9,6 @@ import {CustomElement} from './custom_element.js';
 import {TabElement} from './tab.js';
 import {TabsApiProxy} from './tabs_api_proxy.js';
 
-/** @const {number} */
-const GHOST_PINNED_TAB_COUNT = 3;
-
 /**
  * The amount of padding to leave between the edge of the screen and the active
  * tab when auto-scrolling. This should leave some room to show the previous or
@@ -173,15 +170,6 @@ class TabListElement extends CustomElement {
         this.shadowRoot.querySelector('tabstrip-tab[active]'));
   }
 
-  /**
-   * @return {number}
-   * @private
-   */
-  getPinnedTabsCount_() {
-    return this.pinnedTabsContainerElement_.childElementCount -
-        GHOST_PINNED_TAB_COUNT;
-  }
-
   /**
    * @param {!TabElement} tabElement
    * @param {number} index
@@ -197,12 +185,11 @@ class TabListElement extends CustomElement {
     } else {
       // Pinned tabs are in their own container, so the index of non-pinned
       // tabs need to be offset by the number of pinned tabs
-      const offsetIndex = index - this.getPinnedTabsCount_();
+      const offsetIndex =
+          index - this.pinnedTabsContainerElement_.childElementCount;
       this.tabsContainerElement_.insertBefore(
           tabElement, this.tabsContainerElement_.childNodes[offsetIndex]);
     }
-
-    this.updatePinnedTabsState_();
   }
 
   /**
@@ -234,13 +221,10 @@ class TabListElement extends CustomElement {
       return;
     }
 
-    let dragOverIndex =
-        Array.from(dragOverItem.parentNode.children).indexOf(dragOverItem);
     event.dataTransfer.dropEffect = 'move';
-    if (!dragOverItem.tab.pinned) {
-      dragOverIndex += this.getPinnedTabsCount_();
-    }
 
+    const dragOverIndex =
+        Array.from(dragOverItem.parentNode.children).indexOf(dragOverItem);
     this.tabsApi_.moveTab(this.draggedItem_.tab.id, dragOverIndex);
   }
 
@@ -368,7 +352,6 @@ class TabListElement extends CustomElement {
     if (tabElement) {
       this.addAnimationPromise_(new Promise(async resolve => {
         await tabElement.slideOut();
-        this.updatePinnedTabsState_();
         resolve();
       }));
     }
@@ -436,14 +419,6 @@ class TabListElement extends CustomElement {
       tab.updateThumbnail(imgData);
     }
   }
-
-  /** @private */
-  updatePinnedTabsState_() {
-    this.pinnedTabsContainerElement_.toggleAttribute(
-        'empty',
-        this.pinnedTabsContainerElement_.childElementCount ===
-            GHOST_PINNED_TAB_COUNT);
-  }
 }
 
 customElements.define('tabstrip-tab-list', TabListElement);

chrome/test/data/webui/tab_strip/tab_list_test.js

@@ -219,64 +219,6 @@ suite('TabList', () => {
     assertEquals(unpinnedTabElements[0].tab.id, tabToPin.id);
   });
 
-  test('updates [empty] attribute on container for pinned tabs', async () => {
-    assertTrue(tabList.shadowRoot.querySelector('#pinnedTabsContainer')
-                   .hasAttribute('empty'));
-    const tabToPin = currentWindow.tabs[1];
-    const changeInfo = {index: 0, pinned: true};
-    const updatedTab = Object.assign({}, tabToPin, changeInfo);
-    callbackRouter.onUpdated.dispatchEvent(tabToPin.id, changeInfo, updatedTab);
-    assertFalse(tabList.shadowRoot.querySelector('#pinnedTabsContainer')
-                    .hasAttribute('empty'));
-
-    // Remove the pinned tab
-    callbackRouter.onRemoved.dispatchEvent(
-        tabToPin.id, {windowId: currentWindow.id});
-    await tabList.animationPromises;
-    assertTrue(tabList.shadowRoot.querySelector('#pinnedTabsContainer')
-                   .hasAttribute('empty'));
-  });
-
-  test('shows and hides the ghost pinned tabs based on pinned tabs', () => {
-    const ghostPinnedTabs =
-        tabList.shadowRoot.querySelectorAll('.ghost-pinned-tab');
-    assertEquals(3, ghostPinnedTabs.length);
-
-    // all are hidden by default when there are no pinned tabs
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[0]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[1]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[2]).display);
-
-    // all are visible because 1 pinned tabs leaves room for 3 placeholders
-    pinTabAt(currentWindow.tabs[0], 0);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[0]).display);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[1]).display);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[2]).display);
-
-    // only 2 are visible because 2 pinned tabs leaves room for 2 placeholders
-    pinTabAt(currentWindow.tabs[1], 1);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[0]).display);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[1]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[2]).display);
-
-    // only 2 are visible because 3 pinned tabs leaves room for 1 placeholders
-    pinTabAt(currentWindow.tabs[2], 1);
-    assertEquals('block', window.getComputedStyle(ghostPinnedTabs[0]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[1]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[2]).display);
-
-    // all are hidden because 4 pinned tabs means no room for placeholders
-    callbackRouter.onCreated.dispatchEvent({
-      index: 3,
-      pinned: true,
-      title: 'New pinned tab',
-      windowId: currentWindow.id,
-    });
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[0]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[1]).display);
-    assertEquals('none', window.getComputedStyle(ghostPinnedTabs[2]).display);
-  });
-
   test('moves tab elements when tabs move', () => {
     const tabElementsBeforeMove = getUnpinnedTabs();
     const tabToMove = currentWindow.tabs[0];