Skip to content

Commit 7ff06f9

Browse files
dmaildmail
committed
work
1 parent 7994175 commit 7ff06f9

13 files changed

Lines changed: 588 additions & 69 deletions

File tree

‎packages/frontend/navi/dist/jsenv_navi.js‎

Lines changed: 237 additions & 34 deletions
Large diffs are not rendered by default.

‎packages/frontend/navi/dist/jsenv_navi.js.map‎

Lines changed: 13 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/frontend/navi/docs/MOBILE_LAYOUT_PITFALLS.md‎

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@ or completely invisible.
3131
which triggers the browser's native `scrollIntoView`. So after opening, the page automatically
3232
scrolls to bring the dialog into view — scrolling the user away from where they were.
3333

34-
### The fix: wrap the app content in a scroll container
34+
### The fix: clip the horizontal overflow in a wrapper
3535

3636
The root cause is the document overflowing horizontally. The fix is to **never let the
37-
document itself overflow in X** — instead, contain horizontal scroll inside a child wrapper.
37+
document itself overflow in X** — instead, contain the overflow inside a child wrapper.
3838

3939
```html
4040
<body>
41-
<!-- This wrapper is the scroll container for the whole app -->
42-
<div style="overflow-x: auto;">
41+
<!-- This wrapper absorbs the horizontal overflow of the whole app -->
42+
<div style="overflow-x: clip;">
4343
<!-- all app content goes here -->
4444
</div>
4545

@@ -54,20 +54,74 @@ With this structure:
5454
- `position: fixed; margin: auto` centers the dialog correctly
5555
- No ghost empty space at the bottom
5656

57-
As a safety net, add `overflow-x: hidden` on `html` and `body` to prevent any content
57+
As a safety net, add `overflow-x: clip` on `html` and `body` to prevent any content
5858
that forgets to use a wrapper from inflating the layout viewport:
5959

6060
```css
6161
html,
6262
body {
63-
overflow-x: hidden;
63+
overflow-x: clip;
6464
}
6565
```
6666

67-
Note: `overflow: hidden` on `<html>` does create a new containing block, which could
68-
break `position: fixed` in edge cases — but in practice this safety net is worth having,
69-
and any element that needs correct `position: fixed` behavior (like `<dialog>`) should
70-
be moved to `document.body` directly anyway (which `dialog.jsx` already does).
67+
What it costs: content wider than the screen is cut off instead of reachable by dragging.
68+
When some element genuinely needs to be scrolled horizontally (a wide table, a carousel),
69+
give **that element** its own `overflow-x: auto` — the wrapper stays `clip`.
70+
71+
### Why `clip` and not `auto` or `hidden`
72+
73+
`clip` is the only value that clips without turning the box into a **scroll container**.
74+
`auto`, `scroll` and `hidden` all create one, and that has two consequences that show up
75+
far from the wrapper:
76+
77+
- **Every `position: sticky` in the app sticks to that wrapper**, because sticky resolves
78+
against the nearest scroll container in the DOM — not against the box the app considers
79+
its scroller. The wrapper grows with its content and never scrolls, so nothing sticks
80+
anymore: sticky headers and `<List groupBy>` group labels just scroll away with the
81+
content.
82+
- **Worse than not sticking: the sticky element is offset downwards.** The rectangle a
83+
sticky element sticks within is the scroll container's box shrunk by its
84+
`scroll-padding` (CSS Position L3), and Chromium applies that for an element scroll
85+
container. A wrapper carrying `data-navi-fixed-bar-space` has
86+
`scroll-padding-top: var(--navi-fixed-bar-space-top)`, so labels come to rest at
87+
`scroll-padding-top + top` — a group label floating a hundred pixels below the bar,
88+
covering the content above it.
89+
90+
`overflow-x: clip` also lets `overflow-y` stay `visible`, where `hidden`/`auto` force the
91+
other axis to `auto`.
92+
93+
Note: `overflow: hidden` on `<html>` would additionally create a new containing block,
94+
which could break `position: fixed` in edge cases. Any element that needs correct
95+
`position: fixed` behavior (like `<dialog>`) should be moved to `document.body` directly
96+
anyway (which `dialog.jsx` already does).
97+
98+
### The wrapper is a net, not a fix: find what overflows
99+
100+
Clipping makes the symptom disappear, and with it the signal. Something wider than
101+
the screen is a layout bug wherever it happens — a width in px, a `min-width`, a grid
102+
of fixed columns, an unbreakable string coming from the data. The wrapper only keeps
103+
that bug from taking the whole mobile viewport down with it.
104+
105+
So in dev, ask who overflows:
106+
107+
```js
108+
import { detectHorizontalOverflow } from "@jsenv/navi";
109+
110+
if (import.meta.dev) {
111+
detectHorizontalOverflow({ root: document.querySelector("#main") });
112+
}
113+
```
114+
115+
It outlines the culprits in red and names them in the console, at load and whenever
116+
the layout changes. It reports the **outermost** box that sticks out (its children
117+
stick out because it does), and stays quiet about what cannot reach the document:
118+
anything inside a box that scrolls or clips on its own — a wide table in its own
119+
`overflow-x: auto` container is doing the right thing — and anything `position: fixed`
120+
or in the top layer.
121+
122+
Measuring against the wrapper matters here: once it is in `clip` there is no scrollable
123+
overflow left to read, so `scrollWidth > clientWidth` reports nothing. The rectangles of
124+
the descendants are what tells.
71125
72126
### Also: place `<dialog>` before content in the DOM
73127
@@ -83,7 +137,8 @@ before opening for this reason.
83137
84138
### Summary
85139
86-
| Cause | Symptom | Fix |
87-
| ------------------------------- | ----------------------------------------------------------------- | ------------------------------------------------ |
88-
| Document overflows horizontally | Layout viewport inflated → ghost space below → dialog miscentered | Wrap app content in `overflow-x: auto` container |
89-
| `<dialog>` at end of DOM | Page scrolls to dialog on `showModal()` | Place `<dialog>` first in `<body>` |
140+
| Cause | Symptom | Fix |
141+
| ------------------------------- | ------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------- |
142+
| Document overflows horizontally | Layout viewport inflated → ghost space below → dialog miscentered | Wrap app content in `overflow-x: clip` container |
143+
| `<dialog>` at end of DOM | Page scrolls to dialog on `showModal()` | Place `<dialog>` first in `<body>` |
144+
| Wrapper uses `auto`/`hidden` | It becomes a scroll container: sticky headers and group labels stick to it (and get offset by its `scroll-padding`) | Use `clip`; put `overflow-x: auto` on the wide element itself |

‎packages/frontend/navi/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export { Binder } from "./src/nav/binder/binder.jsx";
9494
export { FixedBar } from "./src/layout/fixed_bar/fixed_bar.jsx";
9595
// debug/tests
9696
export { enableDebugOnDocumentLoading } from "./src/nav/browser_integration/document_loading_signal.js";
97+
export { detectHorizontalOverflow } from "./src/layout/detect_horizontal_overflow.js";
9798

9899
// Details (in between navigation/interaction and fields)
99100
export { Details } from "./src/control/details/details.jsx";

‎packages/frontend/navi/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@jsenv/navi",
3-
"version": "0.29.10",
3+
"version": "0.29.11",
44
"type": "module",
55
"description": "Library of components including navigation to create frontend applications",
66
"repository": {

‎packages/frontend/navi/src/control/list/list.jsx‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ const css = /* css */ `
196196
around it does. Its own scroll box must then be transparent to
197197
layout — otherwise it would cap the list at a height of its own and
198198
start a second, nested scroll inside the page's. */
199-
&[data-scroller="parent"] {
199+
&[data-scroller] {
200200
max-height: none;
201201
overflow: visible;
202202
@@ -206,6 +206,14 @@ const css = /* css */ `
206206
}
207207
}
208208
209+
/* Scrolling with the page means sticking to the viewport, and a FixedBar
210+
is in front of that viewport: without the offset a sticky label lands
211+
behind the bar. The bar publishes the room it takes (see
212+
fixed_bar_space.js) and it is 0px when there is no bar. */
213+
&[data-scroller="document"] {
214+
--x-list-group-label-top: var(--navi-fixed-bar-space-top, 0px);
215+
}
216+
209217
&[data-expand-x] {
210218
width: 100%;
211219
}
@@ -564,7 +572,7 @@ const css = /* css */ `
564572
565573
.navi_list_item_group_label {
566574
position: sticky;
567-
top: 0;
575+
top: var(--list-group-label-top, var(--x-list-group-label-top, 0px));
568576
z-index: 1;
569577
display: block;
570578
background-color: var(--list-group-label-background-color);
@@ -847,7 +855,7 @@ const ListUI = (props) => {
847855
baseClassName="navi_list_container"
848856
popover={popover}
849857
data-horizontal={horizontal ? "" : undefined}
850-
data-scroller={scroller === "self" ? undefined : "parent"}
858+
data-scroller={getScrollerAttribute(scroller)}
851859
data-expand-x={expandX || expand ? "" : undefined}
852860
data-expand-y={expandY || expand ? "" : undefined}
853861
expandX={expandX}
@@ -1127,6 +1135,7 @@ const useListScrollSync = ({
11271135
);
11281136
};
11291137
useLayoutEffect(resolveScroller);
1138+
useStickyScrollportWarning(ref, scroller);
11301139

11311140
// The row the scroll holds onto across a change of geometry, and where it
11321141
// sat when that change was decided. Captured at the two moments the list
@@ -1931,6 +1940,83 @@ const getScrollerViewportRect = (scrollerEl) => {
19311940
}
19321941
return scrollerEl.getBoundingClientRect();
19331942
};
1943+
// What a sticky part of the list sticks to is the nearest scroll container in
1944+
// the DOM; `scroller` has no say in it. A list told the page scrolls it can
1945+
// therefore have its group labels and its header stuck to a wrapper that never
1946+
// scrolls — and pushed down by that wrapper's scroll-padding on top of it. The
1947+
// usual culprit is an app wrapper carrying `overflow-x: auto` to keep the
1948+
// document from overflowing horizontally on mobile; `overflow-x: clip` keeps
1949+
// that guarantee without making a scroll container.
1950+
const STICKY_LIST_PART_SELECTOR = `.navi_list_item_header, .navi_list_item_footer, .navi_list_item_group_label`;
1951+
const useStickyScrollportWarning = (ref, scroller) => {
1952+
const doneRef = useRef(false);
1953+
useLayoutEffect(() => {
1954+
if (!import.meta.dev || doneRef.current || scroller !== "document") {
1955+
return;
1956+
}
1957+
const listContainerEl = ref.current;
1958+
if (!listContainerEl) {
1959+
return;
1960+
}
1961+
const scrollportEl = findScrollportEl(listContainerEl);
1962+
if (!scrollportEl) {
1963+
doneRef.current = true;
1964+
return;
1965+
}
1966+
// Groups arrive with the rows: nothing sticky yet only means "not yet".
1967+
const stickyEl = listContainerEl.querySelector(STICKY_LIST_PART_SELECTOR);
1968+
if (!stickyEl) {
1969+
return;
1970+
}
1971+
doneRef.current = true;
1972+
console.warn(
1973+
`<List scroller="document"> is inside ${getElementSignature(
1974+
scrollportEl,
1975+
)}, a scroll container: its sticky group labels and header stick to that box instead of to the page. Give that box "overflow: clip" (it clips without creating a scroll container), or tell the list about it with scroller={element}.`,
1976+
{ list: listContainerEl, scrollport: scrollportEl, sticky: stickyEl },
1977+
);
1978+
});
1979+
};
1980+
// Anything but visible and clip makes a scroll container, hidden included.
1981+
const isScrollportOverflow = (overflow) =>
1982+
overflow !== "visible" && overflow !== "clip";
1983+
const findScrollportEl = (el) => {
1984+
const { documentElement, body } = document;
1985+
let ancestor = el.parentElement;
1986+
while (ancestor && ancestor !== documentElement) {
1987+
const { overflowX, overflowY } = getComputedStyle(ancestor);
1988+
if (isScrollportOverflow(overflowX) || isScrollportOverflow(overflowY)) {
1989+
// <body> keeps its overflow for itself only when <html> declares one of
1990+
// its own; otherwise it hands it to the viewport, which is the page.
1991+
if (ancestor === body) {
1992+
const rootStyle = getComputedStyle(documentElement);
1993+
if (
1994+
!isScrollportOverflow(rootStyle.overflowX) &&
1995+
!isScrollportOverflow(rootStyle.overflowY)
1996+
) {
1997+
return null;
1998+
}
1999+
}
2000+
return ancestor;
2001+
}
2002+
ancestor = ancestor.parentElement;
2003+
}
2004+
return null;
2005+
};
2006+
2007+
// The CSS needs to tell "the page scrolls me" from "some box around me
2008+
// scrolls me": only the first one sticks to the viewport, where the fixed bars
2009+
// are.
2010+
const getScrollerAttribute = (scroller) => {
2011+
if (scroller === "self") {
2012+
return undefined;
2013+
}
2014+
if (scroller === "document") {
2015+
return "document";
2016+
}
2017+
return "parent";
2018+
};
2019+
19342020
// scroller="parent": the list virtualizes against the scroll box it lives in
19352021
// instead of one of its own. Which box that is can only be measured, and a
19362022
// measurement holds for the geometry it was taken on — see resolveScroller in

0 commit comments

Comments
 (0)