Fix/2014 tabs weird behavior in safari and ff

.changeset/grumpy-shoes-act.md

@@ -0,0 +1,5 @@
+---
+'@sl-design-system/tabs': patch
+---
+
+Fixes selecting tab (and showing the selected tab) for Safari and Firefox (when zooming in/out as well).

packages/components/tabs/src/tab-group.scss

@@ -92,6 +92,7 @@
     overflow: clip scroll;
     overscroll-behavior-y: contain;
     scroll-snap-points-y: repeat(100%);
+    scroll-snap-type: block;
   }
 
   [part='tablist'] {
@@ -162,7 +163,7 @@
   overscroll-behavior-x: contain;
   scroll-behavior: auto;
   scroll-snap-points-x: repeat(100%);
-  scroll-snap-type: x mandatory;
+  scroll-snap-type: inline;
   scrollbar-width: none;
 
   &::-webkit-scrollbar {
@@ -184,13 +185,13 @@
 .indicator {
   background: var(--sl-color-border-selected);
   block-size: var(--sl-size-025);
-  inline-size: 100px; // Default value; actual width depends on scale transform
+  inline-size: 100px; // Default value; actual width depends on calculated value
   inset: auto auto 0 0;
   opacity: 0;
   position: absolute;
-  scale: 0 1;
   transform-origin: center left;
-  transition-property: scale, translate;
+  transition-property:
+    width, height, translate; // TODO: `width` and `height` are not animated on the GPU, but `scale` is not working properly in Safari e.g. when zooming in/out (so we cannot use it right now). Change to `scale` when Safari supports it in a proper way.
   transition-timing-function: ease-in-out;
   translate: 0;
 

packages/components/tabs/src/tab-group.spec.ts

@@ -270,7 +270,7 @@ describe('sl-tab-group', () => {
       `);
 
       // We need to wait for the RovingTabindexController to do its thing
-      await new Promise(resolve => setTimeout(resolve, 50));
+      await new Promise(resolve => setTimeout(resolve, 100));
     });
 
     it('should have a menu button', () => {
@@ -318,7 +318,7 @@ describe('sl-tab-group', () => {
       `);
 
       // We need to wait for the RovingTabindexController to do its thing
-      await new Promise(resolve => setTimeout(resolve, 50));
+      await new Promise(resolve => setTimeout(resolve, 100));
     });
 
     it('should have a menu button', async () => {
@@ -344,7 +344,7 @@ describe('sl-tab-group', () => {
       `);
 
       // We need to wait for the RovingTabindexController to do its thing
-      await new Promise(resolve => setTimeout(resolve, 50));
+      await new Promise(resolve => setTimeout(resolve, 100));
 
       tab = el.querySelector('sl-tab')!;
       link = tab.renderRoot.querySelector('a')!;

packages/components/tabs/src/tab-group.ts

@@ -85,6 +85,9 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
   /** Unique prefix ID for each component in the light DOM. */
   #idPrefix = `sl-tab-group-${nextUniqueId++}`;
 
+  /** Menu element, is shown when the tabs are overflowing. */
+  #menu?: Menu;
+
   /**
    * Observe changes to the selected tab and update accordingly. This observer
    * is necessary for changes to the selected tab that are made programmatically.
@@ -117,6 +120,8 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
     }
 
     this.#mutationObserver?.observe(this, OBSERVER_OPTIONS);
+
+    this.#scrollToTabPanelStart();
   });
 
   /**
@@ -134,6 +139,11 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
     this.#shouldAnimate = false;
     this.#updateSize(hostResized, scrollerResized);
     this.#shouldAnimate = true;
+
+    if (this.selectedTab) {
+      this.#updateSelectedTab(this.selectedTab, false);
+      this.#scrollToTabPanelStart();
+    }
   });
 
   /** Manage keyboard navigation between tabs. */
@@ -149,12 +159,12 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
     listenerScope: (): HTMLElement => this.renderRoot.querySelector('[part="tablist"]') as HTMLElement
   });
 
-  /** Menu element, is shown when the tabs are overflowing. */
-  #menu?: Menu;
-
   /** Determines whether the active tab indicator should animate. */
   #shouldAnimate = false;
 
+  /** Timeout id, to be used with `clearTimeout`. */
+  #timeoutId?: ReturnType<typeof setTimeout>;
+
   /**
    * Determines when the contents of a tab is shown. Auto means the contents will be
    * shown when the tab is focused. Manual means the user has to activate the tab first
@@ -196,17 +206,30 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
    */
   @property({ type: Boolean, reflect: true }) vertical?: boolean;
 
-  override connectedCallback(): void {
-    super.connectedCallback();
+  override disconnectedCallback(): void {
+    if (this.#timeoutId) {
+      clearTimeout(this.#timeoutId);
+      this.#timeoutId = undefined;
+    }
+
+    this.#resizeObserver.disconnect();
+    this.#mutationObserver.disconnect();
+
+    super.disconnectedCallback();
+  }
+
+  override firstUpdated(changes: PropertyValues<this>): void {
+    super.firstUpdated(changes);
 
     this.#mutationObserver.observe(this, OBSERVER_OPTIONS);
 
     // We want to observe the size of the component so we can scroll the selected
     // tab into view if needed.
     this.#resizeObserver.observe(this);
 
-    // We need to wait for the next frame so the element has time to render
-    requestAnimationFrame(() => {
+    // Delay ensures the DOM is fully rendered and layout is stable before running scroll and indicator logic,
+    // improving compatibility with Firefox and preventing visual glitches.
+    this.#timeoutId = setTimeout(() => {
       const scroller = this.renderRoot.querySelector('[part="scroller"]') as HTMLElement;
 
       // Manually trigger the scroll event handler the first time,
@@ -218,14 +241,16 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
       // changes size for example when fonts are loaded. The
       // other elements do not change size while the scroller does.
       this.#resizeObserver.observe(scroller);
-    });
-  }
 
-  override disconnectedCallback(): void {
-    this.#resizeObserver.disconnect();
-    this.#mutationObserver.disconnect();
+      // Manually trigger the scroll event handler the first time,
+      // so that the fade elements are shown if necessary.
+      this.#onScroll(scroller);
 
-    super.disconnectedCallback();
+      if (this.selectedTab) {
+        this.#updateSelectedTab(this.selectedTab, false);
+        this.#scrollToTabPanelStart();
+      }
+    }, 50);
   }
 
   override updated(changes: PropertyValues<this>): void {
@@ -236,18 +261,6 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
       this.#updateSelectionIndicator();
       this.#shouldAnimate = true;
     }
-
-    // In vertical mode, we need to observe the scroller for changes in size to
-    // determine when we need to show the menu button.
-    if (changes.has('vertical')) {
-      const scroller = this.renderRoot.querySelector('[part="scroller"]') as HTMLElement;
-
-      if (this.vertical) {
-        this.#resizeObserver.observe(scroller);
-      } else {
-        this.#resizeObserver.unobserve(scroller);
-      }
-    }
   }
 
   override render(): TemplateResult {
@@ -349,6 +362,11 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
 
     this.toggleAttribute('scroll-start', scrollStart);
     this.toggleAttribute('scroll-end', scrollEnd);
+
+    // Keep the indicator aligned while the scroller moves
+    if (this.selectedTab) {
+      this.#updateSelectionIndicator();
+    }
   }
 
   #onTabSlotChange(event: Event & { target: HTMLSlotElement }): void {
@@ -360,6 +378,7 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
     const selectedTab = this.tabs.find(tab => tab.selected);
     if (selectedTab) {
       this.#updateSelectedTab(selectedTab, false);
+      this.#scrollToTabPanelStart();
     }
 
     this.#rovingTabindexController.clearElementCache();
@@ -404,18 +423,30 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
     if (this.vertical) {
       if (tabRect.top < scrollerRect.top) {
         // The tab is above the top edge of the scroller
-        scroller.scrollBy({ top: tabRect.top - scrollerRect.top, behavior });
+        scroller.scrollTo({
+          top: scroller.scrollTop + (tabRect.top - scrollerRect.top),
+          behavior
+        });
       } else if (tabRect.bottom > scrollerRect.bottom) {
         // The tab is below the bottom edge of the scroller
-        scroller.scrollBy({ top: tabRect.bottom - scrollerRect.bottom, behavior });
+        scroller.scrollTo({
+          top: scroller.scrollTop + (tabRect.bottom - scrollerRect.bottom),
+          behavior
+        });
       }
     } else {
       if (tabRect.left < scrollerRect.left) {
         // The tab is to the left of the left edge of the scroller
-        scroller.scrollBy({ left: tabRect.left - scrollerRect.left, behavior });
+        scroller.scrollTo({
+          left: scroller.scrollLeft + (tabRect.left - scrollerRect.left),
+          behavior
+        });
       } else if (tabRect.right > scrollerRect.right) {
         // The tab is to the right of the right edge of the scroller
-        scroller.scrollBy({ left: tabRect.right - scrollerRect.right, behavior });
+        scroller.scrollTo({
+          left: scroller.scrollLeft + (tabRect.right - scrollerRect.right),
+          behavior
+        });
       }
     }
   }
@@ -428,7 +459,12 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
 
     // Scroll to make sure the top of the panel is visible, but don't scroll too far
     // so the tab container/wrapper may become unstuck.
-    getScrollParent(this)?.scrollBy({ top: top - (this.vertical ? wrapperTop : containerBottom) });
+    const scrollParent = getScrollParent(this);
+    if (scrollParent) {
+      scrollParent.scrollTo({
+        top: scrollParent.scrollTop + top - (this.vertical ? wrapperTop : containerBottom)
+      });
+    }
   }
 
   #updateSelectedTab(selectedTab?: Tab, emitEvent = true): void {
@@ -450,6 +486,10 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
 
     if (selectedTab) {
       this.#scrollIntoViewIfNeeded(selectedTab, emitEvent ? 'smooth' : 'instant');
+
+      requestAnimationFrame(() => {
+        this.#updateSelectionIndicator();
+      });
     }
   }
 
@@ -458,32 +498,48 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
 
     if (!this.selectedTab) {
       indicator.style.opacity = '';
-      indicator.style.scale = '';
       indicator.style.transitionDuration = '0s';
       indicator.style.translate = '';
+      indicator.style.inlineSize = '';
+      indicator.style.blockSize = '';
 
       return;
     }
 
-    const tablist = this.renderRoot.querySelector('[part="tablist"]') as HTMLElement,
-      rect = this.selectedTab.getBoundingClientRect();
-
     let start = 0;
+
+    const tab = this.selectedTab,
+      scroller = this.renderRoot.querySelector('[part="scroller"]') as HTMLElement;
+
+    if (!tab || !scroller) {
+      return;
+    }
+
+    // Baseline (first tab) so start = 0 for the first tab even when tabs are centered/end aligned
+    const firstTab = this.tabs?.[0],
+      baseInline = firstTab ? firstTab.offsetLeft : 0,
+      baseBlock = firstTab ? firstTab.offsetTop : 0;
+
     if (this.vertical) {
-      start = rect.top - tablist.getBoundingClientRect().top;
+      start = tab.offsetTop - baseBlock;
     } else {
-      start = rect.left - tablist.getBoundingClientRect().left;
+      start = tab.offsetLeft - baseInline;
     }
 
     indicator.style.opacity = '1';
     indicator.style.transitionDuration = this.#shouldAnimate ? '' : '0s';
     indicator.style.transitionProperty = indicator.style.translate === '' ? 'opacity' : '';
 
+    const sizeInline = tab.offsetWidth,
+      sizeBlock = tab.offsetHeight;
+
     if (this.vertical) {
-      indicator.style.scale = `1 ${rect.height / 100}`;
+      indicator.style.blockSize = `${sizeBlock}px`;
+      indicator.style.inlineSize = '';
       indicator.style.translate = `0 ${start}px`;
     } else {
-      indicator.style.scale = `${rect.width / 100} 1`;
+      indicator.style.inlineSize = `${sizeInline}px`;
+      indicator.style.blockSize = '';
       indicator.style.translate = `${start}px`;
     }
   }
@@ -526,10 +582,10 @@ export class TabGroup extends ScopedElementsMixin(LitElement) {
       // menu button *will* trigger that callback. If we don't, then the selected tab
       // may not be fully visible.
       if (showingMenu === this.showMenu && this.selectedTab) {
-        this.#scrollIntoViewIfNeeded(this.selectedTab, 'instant');
+        this.#scrollIntoViewIfNeeded(this.selectedTab, 'auto');
       }
     } else if (hostResized && this.selectedTab) {
-      this.#scrollIntoViewIfNeeded(this.selectedTab, 'instant');
+      this.#scrollIntoViewIfNeeded(this.selectedTab, 'auto');
     }
 
     this.#updateSelectionIndicator();

packages/components/tabs/src/tab.spec.ts

@@ -1,10 +1,13 @@
+import { setupIgnoreWindowResizeObserverLoopErrors } from '@lit-labs/virtualizer/support/resize-observer-errors.js';
 import { expect, fixture } from '@open-wc/testing';
 import { sendKeys } from '@web/test-runner-commands';
 import { html } from 'lit';
 import { spy } from 'sinon';
 import '../register.js';
 import { Tab } from './tab.js';
 
+setupIgnoreWindowResizeObserverLoopErrors(beforeEach, afterEach, { suppressErrorLogging: true });
+
 describe('sl-tab', () => {
   let el: Tab;