Skip to content

Commit

Permalink
Fix Chromium scrollbar-induced layout shifts
Browse files Browse the repository at this point in the history
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 the 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 browser on Linux/Windows:

1. Open the app with a height large enough where a 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 a CSS mixin 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 test to avoid regression.
- Update DevToolkit to accommodate new scrollbar spacing.
  • Loading branch information
undergroundwires committed May 9, 2024
1 parent dd71536 commit bc4879c
Show file tree
Hide file tree
Showing 12 changed files with 377 additions and 180 deletions.
5 changes: 5 additions & 0 deletions src/presentation/assets/styles/base/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
11 changes: 9 additions & 2 deletions src/presentation/components/DevToolkit/DevToolkit.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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[] = [
{
Expand All @@ -58,6 +60,7 @@ export default defineComponent({
devActions,
isOpen,
close,
scrollbarGutterWidth,
};
},
});
Expand All @@ -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;
Expand Down
39 changes: 39 additions & 0 deletions src/presentation/components/DevToolkit/UseScrollbarGutterWidth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
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, () => {
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,6 @@ export interface ScrollDomStateAccessor {
readonly htmlScrollHeight: number;
readonly htmlClientWidth: number;
readonly htmlClientHeight: number;
readonly htmlOffsetWidth: number;
readonly htmlOffsetHeight: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -206,13 +205,21 @@ 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,
calculateScrollbarGutterStyle(initialState.htmlClientWidth, initialState.htmlOffsetWidth),
],
);
return ScrollRevertAction.RestoreRequired;
},
Expand All @@ -223,13 +230,21 @@ 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,
calculateScrollbarGutterStyle(initialState.htmlClientHeight, initialState.htmlOffsetHeight),
],
);
return ScrollRevertAction.RestoreRequired;
},
Expand Down Expand Up @@ -280,11 +295,20 @@ interface DomPropertyMutator<TInitialStateValue> {
restoreStateOnUnblock(storedState: TInitialStateValue, dom: ScrollDomStateAccessor): void;
}

function calculateBodyViewportStyleWithMargins(
margins: readonly string[],
/** Calculates allocated scrollbar gutter, adjusting for `scrollbar-gutter: stable` */
function calculateScrollbarGutterStyle(
clientSize: number,
offsetSize: number,
): string {
const scrollbarGutterSize = clientSize - offsetSize;
return scrollbarGutterSize !== 0 ? `${scrollbarGutterSize}px` : '';
}

function calculateAdjustedStyle(
spaceOffsets: readonly string[],
): string {
let value = '100%';
const calculatedMargin = margins
const calculatedMargin = spaceOffsets
.filter((marginText) => marginText.length > 0)
.join(' + '); // without setting margins, it leads to layout shift if body has margin
if (calculatedMargin) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
16 changes: 15 additions & 1 deletion tests/e2e/no-unintended-layout-shifts.cy.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 });
});
});
30 changes: 27 additions & 3 deletions tests/e2e/support/assert/layout-stability.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
]));
}
});
}

Expand Down
18 changes: 15 additions & 3 deletions tests/e2e/support/scenarios/viewport-test-scenarios.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
Loading

0 comments on commit bc4879c

Please sign in to comment.