lxbme opened a new pull request, #2096:
URL: https://github.com/apache/apisix-website/pull/2096
Changes:
Two layout defects, both in `next/src/styles/global.css`.
### 1. Body text ran flush against the viewport edge
`.container` sets `padding: 0 1.25rem`, but `.article-wrap` and
`.docs-layout` are applied to the **same elements** (`class="container
article-wrap"`, `class="container docs-layout"`) and re-declared the `padding`
**shorthand** later in the sheet:
```css
.container { … padding: 0 1.25rem } /* line 38 */
.article-wrap { … padding: 2.5rem 0 } /* line 209 — resets the inline
axis */
.docs-layout { … padding: 2rem 0 } /* line 324 — same */
```
Same specificity, later rule wins, so the horizontal padding became `0`.
Measured on production: the article `<h1>` sat at `x=0` while the header brand
— which was always correct — sat at `x=20`.
This affected **every viewport narrower than the 1140px container cap**, and
not only docs: `Article.astro` composes its class with a template literal, so
blog posts, articles and the Learning Center share the defect.
Both rules now declare `padding-block` and leave the inline axis to
`.container`. Restating `padding: 2rem 1.25rem` would have copied
`.container`'s value into two more places, free to drift; the bug was a
shorthand overriding an axis it had no opinion about, so the fix is to stop
expressing one. Logical properties are already used in this sheet
(`margin-inline`, line 447).
### 2. The mobile docs nav sat about seven screens below the article
`@media (max-width: 960px)` set `.docs-sidebar{order: 2}`. Measured on a
900px-wide viewport:
- the nav began at **4743px of a 5566px page** — 7.3 screens down;
- its `45vh` box then showed **4.1%** of its 7090px of content, about 7 of
200 links;
- the version picker lives inside that box, so switching doc versions cost
the same 7.3 screens.
The DOM already places `<nav>` before `<article>`; only that `order` moved
it. Dropping it restores document order, and the cap goes 45vh → 30vh so the
article's own heading still lands on the first screen.
**Why the nav is repositioned rather than collapsed.** A `<details>`
disclosure was the first design. It cannot work: `<details>`'s open state is a
single DOM attribute, and a media query cannot set it — mobile needs it closed
while desktop needs it open. Two CSS workarounds were prototyped and both
failed in Chrome 150, verified with `Element.checkVisibility()`: forcing the
content `display:block` with the summary hidden, and giving the `<details>`
itself `display:contents`. Browsers do not hide collapsed `<details>` content
with `display:none`, so author CSS cannot override it.
(`getComputedStyle().display` reports `block` in both cases and is misleading;
only `checkVisibility()` reflects reality.) A checkbox-hack collapse did work
in all four states, but a screen reader announces it as a checkbox rather than
a disclosure control. Repositioning reaches the same goal with no markup
change, no JavaScript, and no semantics traded away.
### Deliberate costs
Restoring the inline padding narrows each grid's content box by 2.5rem:
| Grid | Before | After |
|---|---|---|
| `.docs-layout` | content column 820px | 780px |
| `.article-wrap` (<1240px) | 720px cap | unchanged |
| `.article-wrap.with-rails` (≥1240px) | needs 1276px, had 1340px | has
1300px (headroom 64px → 24px) |
At exactly 1240px the reading column goes 724px → 684px. That rule's own
comment predicts "~680px at 1240", so this stays inside the author's stated
envelope.
### Testing
`next/tests/e2e/docs-mobile-layout.spec.mjs` asserts computed layout values
on both the desktop (1440×900) and mobile (390×844) Playwright projects.
Three of the four tests are **ungated and run in PR CI**: `docs/general/**`
and blog posts both ship from this repo, so they exist once `sync-content.mjs`
has run — which is what `lint.yml` does. Only the `docs/apisix/**` case is
gated on `EXPECT_DOCUSARUS_ROUTES`, because that tree needs `.sync/` checkouts
only the deploy pipeline has.
Two assertions are deliberately shaped against false greens:
- The nav-order check is scoped to ≤960px. Above it, the nav and article are
grid items on the same row with **equal** `offsetTop` (measured: both 132 at
1440px), so the desktop branch asserts equality instead — which is what catches
`order` escaping the media query.
- The rails check asserts the reading column's **width**, not the track
count. `.with-rails` uses an explicit template, so computed
`gridTemplateColumns` always reports three tracks: forcing the wrapper to 600px
still reports three, as `190px 44px 230px`, with the reading column crushed.
Verified red against production before the fix, and green locally against a
full 875-page build (5 passed / 3 skipped; the apisix test skips without the
env gate, the rails test skips on mobile by design).
Honest limits: the apisix-docs assertion will first actually execute in the
deploy pipeline, not in PR CI. And `max-height: 30vh` is a chosen value
asserted by no test — a test asserting it would only mirror the CSS and calcify
a tunable.
### Merge order
No `.astro` file is touched, so this and #2095 merge cleanly in either
order. Verified with `git merge-tree` against both `upstream/master` and that
branch.
Screenshots of the change:
Mobile, before: article text flush against both edges, docs nav 7.3 screens
below the article with 4.1% of the link tree visible. After: 20px inline
padding matching the header, nav above the article in a 30vh scrolling box.
Desktop is unchanged apart from the intended 40px inset.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
