From 5cf3e6f8d6706476b938970c39f7a3169ff33754 Mon Sep 17 00:00:00 2001
From: undergroundwires <git@undergroundwires.dev>
Date: Thu, 9 May 2024 14:33:57 +0200
Subject: [PATCH] Fix Chromium scrollbar-induced layout shifts

This commit addresses an issue in Chromium on Linux and Windows where
the appearance of a vertical scrollbar causes unexpected horizontal
layout shifts. This behavior typically occurs when window is resized, a
card is opened or a script is selected, resulting in content being
pushed to the left.

The solution implemented involves using `scrollbar-gutter: stable` to
ensure space is always allocated for the scrollbar, thus preventing any
shift in the page layout. This fix primarily affects Chromium-based
browsers on Linux and Windows. It has no impact on Firefox on any
platform, or any browser on macOS (including Chromium). Because these
render the scrollbar as an overlay, and do not suffer from this issue.

Steps to reproduce the issue using Chromium brower on Linux/Windows:

1. Open the app width big enough height where vertical scrollbar is not
   visible.
2. Resize the window to a height that triggers a vertical scrollbar.
3. Notice the layout shift as the body content moves to the right.

Changes:

- Add CSS mixins and styles to handle scrollbar gutter allocation with a
  fallback.
- Add support for modal dialog background lock to handle
  `scrollbar-gutter: stable;` in calculations to avoid layout shift when
  a modal is open.
- Add E2E tost to avoid regression.
- Update DevToolkit to accommodate new scrollbar spacing.
---
 src/presentation/assets/styles/_mixins.scss   |  4 ++
 .../assets/styles/base/_index.scss            |  5 ++
 .../base/_prevent-scrollbar-layout-shift.scss | 19 ++++++++
 .../components/DevToolkit/DevToolkit.vue      | 11 ++++-
 .../DevToolkit/UseScrollbarGutterWidth.ts     | 40 ++++++++++++++++
 .../ScrollLock/ScrollDomStateAccessor.ts      |  2 +
 .../ScrollLock/UseLockBodyBackgroundScroll.ts | 46 +++++++++++++++----
 .../WindowScrollDomStateAccessor.ts           |  4 ++
 tests/e2e/no-unintended-layout-shifts.cy.ts   | 16 ++++++-
 tests/e2e/support/assert/layout-stability.ts  | 30 ++++++++++--
 .../scenarios/viewport-test-scenarios.ts      | 18 ++++++--
 11 files changed, 178 insertions(+), 17 deletions(-)
 create mode 100644 src/presentation/assets/styles/base/_prevent-scrollbar-layout-shift.scss
 create mode 100644 src/presentation/components/DevToolkit/UseScrollbarGutterWidth.ts

diff --git a/src/presentation/assets/styles/_mixins.scss b/src/presentation/assets/styles/_mixins.scss
index 20211bc26..1c55ba5c2 100644
--- a/src/presentation/assets/styles/_mixins.scss
+++ b/src/presentation/assets/styles/_mixins.scss
@@ -133,3 +133,7 @@
   font-family: $font-family-main;
   font-size: $font-size-absolute-normal;
 }
+
+@mixin allocate-scrollbar-space {
+
+}
diff --git a/src/presentation/assets/styles/base/_index.scss b/src/presentation/assets/styles/base/_index.scss
index 45edc2bb6..081a99b63 100644
--- a/src/presentation/assets/styles/base/_index.scss
+++ b/src/presentation/assets/styles/base/_index.scss
@@ -13,11 +13,16 @@
 @use "_code-styling" as *;
 @use "_margin-padding" as *;
 @use "_link-styling" as *;
+@use "_prevent-scrollbar-layout-shift" as *;
 
 * {
   box-sizing: border-box;
 }
 
+html {
+  @include prevent-scrollbar-layout-shift;
+}
+
 body {
   background: $color-background;
   @include base-font-style;
diff --git a/src/presentation/assets/styles/base/_prevent-scrollbar-layout-shift.scss b/src/presentation/assets/styles/base/_prevent-scrollbar-layout-shift.scss
new file mode 100644
index 000000000..73796c538
--- /dev/null
+++ b/src/presentation/assets/styles/base/_prevent-scrollbar-layout-shift.scss
@@ -0,0 +1,19 @@
+// This mixin prevents layout shifts caused by the appearance of a vertical scrollbar
+// in Chromium-based browsers on Linux and Windows.
+// It creates a reserved space for the scrollbar, ensuring content remains stable and does
+// not shift horizontally when the scrollbar appears.
+@mixin prevent-scrollbar-layout-shift {
+  scrollbar-gutter: stable;
+
+  @supports not (scrollbar-gutter: stable) { // https://caniuse.com/mdn-css_properties_scrollbar-gutter
+    // Safari workaround: Shift content to accommodate non-overlay scrollbar.
+    // An issue: On small screens, the appearance of the scrollbar can shift content, due to limited space for
+    // both content and scrollbar.
+    $full-width-including-scrollbar: 100vw;
+    $full-width-excluding-scrollbar: 100%;
+    $scrollbar-width: calc($full-width-including-scrollbar - $full-width-excluding-scrollbar);
+    padding-inline-start: $scrollbar-width; // Allows both right-to-left (RTL) and left-to-right (LTR) text direction support
+  }
+
+  // More details: https://web.archive.org/web/20240509122237/https://stackoverflow.com/questions/1417934/how-to-prevent-scrollbar-from-repositioning-web-page
+}
diff --git a/src/presentation/components/DevToolkit/DevToolkit.vue b/src/presentation/components/DevToolkit/DevToolkit.vue
index dbdd5510e..b130bf8c4 100644
--- a/src/presentation/components/DevToolkit/DevToolkit.vue
+++ b/src/presentation/components/DevToolkit/DevToolkit.vue
@@ -31,6 +31,7 @@ import { defineComponent, ref } from 'vue';
 import { injectKey } from '@/presentation/injectionSymbols';
 import FlatButton from '@/presentation/components/Shared/FlatButton.vue';
 import { dumpNames } from './DumpNames';
+import { useScrollbarGutterWidth } from './UseScrollbarGutterWidth';
 
 export default defineComponent({
   components: {
@@ -39,6 +40,7 @@ export default defineComponent({
   setup() {
     const { log } = injectKey((keys) => keys.useLogger);
     const isOpen = ref(true);
+    const scrollbarGutterWidth = useScrollbarGutterWidth();
 
     const devActions: readonly DevAction[] = [
       {
@@ -58,6 +60,7 @@ export default defineComponent({
       devActions,
       isOpen,
       close,
+      scrollbarGutterWidth,
     };
   },
 });
@@ -71,10 +74,14 @@ interface DevAction {
 <style scoped lang="scss">
 @use "@/presentation/assets/styles/main" as *;
 
+$viewport-edge-offset: $spacing-absolute-large; // close to Chromium gutter width (15px)
+
 .dev-toolkit-container {
   position: fixed;
-  top: 0;
-  right: 0;
+
+  top: $viewport-edge-offset;
+  right: max(v-bind(scrollbarGutterWidth), $viewport-edge-offset);
+
   background-color: rgba($color-on-surface, 0.5);
   color: $color-on-primary;
   padding: $spacing-absolute-medium;
diff --git a/src/presentation/components/DevToolkit/UseScrollbarGutterWidth.ts b/src/presentation/components/DevToolkit/UseScrollbarGutterWidth.ts
new file mode 100644
index 000000000..ff4b0f291
--- /dev/null
+++ b/src/presentation/components/DevToolkit/UseScrollbarGutterWidth.ts
@@ -0,0 +1,40 @@
+import {
+  computed, readonly, ref, watch,
+} from 'vue';
+import { throttle } from '@/application/Common/Timing/Throttle';
+import { useAutoUnsubscribedEventListener } from '../Shared/Hooks/UseAutoUnsubscribedEventListener';
+
+const RESIZE_EVENT_THROTTLE_MS = 200;
+
+export function useScrollbarGutterWidth() {
+  const scrollbarWidthInPx = ref(getScrollbarGutterWidth());
+
+  const { startListening } = useAutoUnsubscribedEventListener();
+  startListening(window, 'resize', throttle(() => {
+    scrollbarWidthInPx.value = getScrollbarGutterWidth();
+  }, RESIZE_EVENT_THROTTLE_MS));
+
+  const bodyWidth = useBodyWidth();
+  watch(() => bodyWidth.value, (b) => {
+    console.log(b);
+    scrollbarWidthInPx.value = getScrollbarGutterWidth();
+  }, { immediate: false });
+
+  const scrollbarWidthStyle = computed(() => `${scrollbarWidthInPx.value}px`);
+  return readonly(scrollbarWidthStyle);
+}
+
+function getScrollbarGutterWidth(): number {
+  return document.documentElement.clientWidth - document.documentElement.offsetWidth;
+}
+
+function useBodyWidth() {
+  const width = ref(document.body.offsetWidth);
+  const observer = new ResizeObserver((entries) => throttle(() => {
+    for (const entry of entries) {
+      width.value = entry.borderBoxSize[0].inlineSize;
+    }
+  }, RESIZE_EVENT_THROTTLE_MS));
+  observer.observe(document.body, { box: 'border-box' });
+  return readonly(width);
+}
diff --git a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/ScrollDomStateAccessor.ts b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/ScrollDomStateAccessor.ts
index fcf67676e..c4c820bd5 100644
--- a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/ScrollDomStateAccessor.ts
+++ b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/ScrollDomStateAccessor.ts
@@ -16,4 +16,6 @@ export interface ScrollDomStateAccessor {
   readonly htmlScrollHeight: number;
   readonly htmlClientWidth: number;
   readonly htmlClientHeight: number;
+  readonly htmlOffsetWidth: number;
+  readonly htmlOffsetHeight: number;
 }
diff --git a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/UseLockBodyBackgroundScroll.ts b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/UseLockBodyBackgroundScroll.ts
index 50b8dfb3b..8534df652 100644
--- a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/UseLockBodyBackgroundScroll.ts
+++ b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/UseLockBodyBackgroundScroll.ts
@@ -103,7 +103,6 @@ const BodyStyleLeft: DomPropertyMutator<{
   storeInitialState: (dom) => ({
     htmlScrollLeft: dom.htmlScrollLeft,
     bodyStyleLeft: dom.bodyStyleLeft,
-    bodyMarginLeft: dom.bodyComputedMarginLeft,
   }),
   onBlock: (initialState, dom) => {
     if (initialState.htmlScrollLeft === 0) {
@@ -206,13 +205,28 @@ const BodyPositionFixed: DomPropertyMutator<{
 
 const BodyWidth100Percent: DomPropertyMutator<{
   readonly bodyStyleWidth: string;
+  readonly htmlOffsetWidth: number;
+  readonly htmlClientWidth: number;
 }> = {
   storeInitialState: (dom) => ({
     bodyStyleWidth: dom.bodyStyleWidth,
+    htmlOffsetWidth: dom.htmlOffsetWidth,
+    htmlClientWidth: dom.htmlClientWidth,
   }),
-  onBlock: (_, dom) => {
-    dom.bodyStyleWidth = calculateBodyViewportStyleWithMargins(
-      [dom.bodyComputedMarginLeft, dom.bodyComputedMarginRight],
+  onBlock: (initialState, dom) => {
+    dom.bodyStyleWidth = calculateAdjustedStyle(
+      [
+        dom.bodyComputedMarginLeft,
+        dom.bodyComputedMarginRight,
+        ...(() => {
+          const scrollbarGutterWidth = initialState.htmlClientWidth
+            - initialState.htmlOffsetWidth;
+          if (scrollbarGutterWidth !== 0) {
+            return [`${scrollbarGutterWidth}px`];
+          }
+          return [];
+        })(),
+      ],
     );
     return ScrollRevertAction.RestoreRequired;
   },
@@ -223,13 +237,28 @@ const BodyWidth100Percent: DomPropertyMutator<{
 
 const BodyHeight100Percent: DomPropertyMutator<{
   readonly bodyStyleHeight: string;
+  readonly htmlOffsetHeight: number;
+  readonly htmlClientHeight: number;
 }> = {
   storeInitialState: (dom) => ({
     bodyStyleHeight: dom.bodyStyleHeight,
+    htmlOffsetHeight: dom.htmlOffsetHeight,
+    htmlClientHeight: dom.htmlClientHeight,
   }),
-  onBlock: (_, dom) => {
-    dom.bodyStyleHeight = calculateBodyViewportStyleWithMargins(
-      [dom.bodyComputedMarginTop, dom.bodyComputedMarginBottom],
+  onBlock: (initialState, dom) => {
+    dom.bodyStyleHeight = calculateAdjustedStyle(
+      [
+        dom.bodyComputedMarginTop,
+        dom.bodyComputedMarginBottom,
+        ...(() => {
+          const scrollbarGutterHeight = initialState.htmlClientHeight
+            - initialState.htmlOffsetHeight;
+          if (scrollbarGutterHeight !== 0) {
+            return [`${scrollbarGutterHeight}px`];
+          }
+          return [];
+        })(),
+      ],
     );
     return ScrollRevertAction.RestoreRequired;
   },
@@ -249,6 +278,7 @@ const ScrollLockMutators: readonly ScrollStateManipulator[] = [
     2. `overscrollBehavior`: Only stops scrolling at scroll limits, not suitable for all cases.
     3. `touchAction: none`: Ineffective on non-touch (desktop) devices.
   */
+  // createScrollStateManipulator(HtmlStyleScrollbarGutter), // Or it messes up things
   ...[ // Keep the scrollbar visible
     createScrollStateManipulator(BodyStyleOverflowX), // Horizontal scrollbar
     createScrollStateManipulator(BodyStyleOverflowY), // Vertical scrollbar
@@ -280,7 +310,7 @@ interface DomPropertyMutator<TInitialStateValue> {
   restoreStateOnUnblock(storedState: TInitialStateValue, dom: ScrollDomStateAccessor): void;
 }
 
-function calculateBodyViewportStyleWithMargins(
+function calculateAdjustedStyle(
   margins: readonly string[],
 ): string {
   let value = '100%';
diff --git a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/WindowScrollDomStateAccessor.ts b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/WindowScrollDomStateAccessor.ts
index 7af22c9f0..0b121b215 100644
--- a/src/presentation/components/Shared/Modal/Hooks/ScrollLock/WindowScrollDomStateAccessor.ts
+++ b/src/presentation/components/Shared/Modal/Hooks/ScrollLock/WindowScrollDomStateAccessor.ts
@@ -61,4 +61,8 @@ class WindowScrollDomState implements ScrollDomStateAccessor {
   get htmlClientWidth(): number { return HtmlElement.clientWidth; }
 
   get htmlClientHeight(): number { return HtmlElement.clientHeight; }
+
+  get htmlOffsetWidth(): number { return HtmlElement.offsetWidth; }
+
+  get htmlOffsetHeight(): number { return HtmlElement.offsetHeight; }
 }
diff --git a/tests/e2e/no-unintended-layout-shifts.cy.ts b/tests/e2e/no-unintended-layout-shifts.cy.ts
index ab7c544b7..aff76e044 100644
--- a/tests/e2e/no-unintended-layout-shifts.cy.ts
+++ b/tests/e2e/no-unintended-layout-shifts.cy.ts
@@ -1,4 +1,4 @@
-import { ViewportTestScenarios } from './support/scenarios/viewport-test-scenarios';
+import { ViewportTestScenarios, LargeScreen } from './support/scenarios/viewport-test-scenarios';
 import { openCard } from './support/interactions/card';
 import { selectAllScripts, unselectAllScripts } from './support/interactions/script-selection';
 import { assertLayoutStability } from './support/assert/layout-stability';
@@ -62,4 +62,18 @@ describe('Layout stability', () => {
       });
     });
   });
+
+  // Regression test for bug on Chromium where horizontal scrollbar visibility causes layout shifts.
+  it('Scrollbar visibility', () => {
+    // arrange
+    cy.viewport(LargeScreen.width, LargeScreen.height);
+    cy.visit('/');
+    openCard({
+      cardIndex: 0,
+    });
+    // act
+    assertLayoutStability('.app__wrapper', () => {
+      cy.viewport(LargeScreen.width, 100); // Set small height to trigger horizontal scrollbar.
+    }, { excludeHeight: true });
+  });
 });
diff --git a/tests/e2e/support/assert/layout-stability.ts b/tests/e2e/support/assert/layout-stability.ts
index 17dc26f60..59ae24902 100644
--- a/tests/e2e/support/assert/layout-stability.ts
+++ b/tests/e2e/support/assert/layout-stability.ts
@@ -1,7 +1,19 @@
 import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage';
 
-export function assertLayoutStability(selector: string, action: ()=> void): void {
+interface LayoutStabilityTestOptions {
+  excludeWidth: boolean;
+  excludeHeight: boolean;
+}
+
+export function assertLayoutStability(
+  selector: string,
+  action: ()=> void,
+  options: Partial<LayoutStabilityTestOptions> | undefined = undefined,
+): void {
   // arrange
+  if (options?.excludeWidth === true && options?.excludeHeight === true) {
+    throw new Error('Invalid test configuration: both width and height exclusions specified.');
+  }
   let initialMetrics: ViewportMetrics | undefined;
   captureViewportMetrics(selector, (metrics) => {
     initialMetrics = metrics;
@@ -11,10 +23,22 @@ export function assertLayoutStability(selector: string, action: ()=> void): void
   // assert
   captureViewportMetrics(selector, (metrics) => {
     const finalMetrics = metrics;
-    expect(initialMetrics).to.deep.equal(finalMetrics, formatAssertionMessage([
+    const assertionContext = [
       `Expected (initial metrics before action): ${JSON.stringify(initialMetrics)}`,
       `Actual (final metrics after action): ${JSON.stringify(finalMetrics)}`,
-    ]));
+    ];
+    if (options?.excludeWidth !== true) {
+      expect(initialMetrics?.x).to.equal(finalMetrics.x, formatAssertionMessage([
+        'Width instability detected',
+        ...assertionContext,
+      ]));
+    }
+    if (options?.excludeHeight !== true) {
+      expect(initialMetrics?.x).to.equal(finalMetrics.x, formatAssertionMessage([
+        'Height instability detected',
+        ...assertionContext,
+      ]));
+    }
   });
 }
 
diff --git a/tests/e2e/support/scenarios/viewport-test-scenarios.ts b/tests/e2e/support/scenarios/viewport-test-scenarios.ts
index 306eb9de7..57be67b08 100644
--- a/tests/e2e/support/scenarios/viewport-test-scenarios.ts
+++ b/tests/e2e/support/scenarios/viewport-test-scenarios.ts
@@ -1,7 +1,19 @@
+const SmallScreen: ViewportScenario = {
+  name: 'iPhone SE', width: 375, height: 667,
+};
+
+const MediumScreen: ViewportScenario = {
+  name: '13-inch Laptop', width: 1280, height: 800,
+};
+
+export const LargeScreen: ViewportScenario = {
+  name: '4K Ultra HD Desktop', width: 3840, height: 2160,
+};
+
 export const ViewportTestScenarios: readonly ViewportScenario[] = [
-  { name: 'iPhone SE', width: 375, height: 667 },
-  { name: '13-inch Laptop', width: 1280, height: 800 },
-  { name: '4K Ultra HD Desktop', width: 3840, height: 2160 },
+  SmallScreen,
+  MediumScreen,
+  LargeScreen,
 ] as const;
 
 interface ViewportScenario {