Skip to content

Commit ed75321

Browse files
committed
docs(gallery-web): add openspec change for pagination placement
1 parent 8d55b97 commit ed75321

6 files changed

Lines changed: 419 additions & 0 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-08-17
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# Design
2+
3+
## Context
4+
5+
Pagination in Gallery renders inside a three-zone flex bar. Both bars are built the same way, except the top bar has no middle zone:
6+
7+
```
8+
.widget-gallery-footer-controls display:flex; row nowrap
9+
├─ .widget-gallery-fc-start grow:1 basis:33.33% selection counter
10+
├─ .widget-gallery-fc-middle (no flex rules today) Load more button
11+
└─ .widget-gallery-fc-end grow:1 basis:33.33% pagination / custom pagination
12+
justify-content: flex-end
13+
14+
.widget-gallery-top-bar-controls
15+
├─ .widget-gallery-tb-start grow:1 basis:33.33% selection counter
16+
└─ .widget-gallery-tb-end grow:1 basis:33.33% pagination
17+
justify-content: flex-end
18+
```
19+
20+
The zone, not the bar, decides horizontal position. The old design property predates this structure: before the overhaul, `.widget-gallery-pagination` was a full-width row of its own directly under `.widget-gallery`, so justifying the bar inside it produced real left/centre alignment. That element is gone, which is why the property is inert.
21+
22+
Occupancy is dynamic:
23+
24+
| occupant | zone | condition |
25+
| ----------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------- |
26+
| selection counter | `*-start` | `selectionCountPosition` matches the bar (defaults to `bottom`) **and** `selectedCount > 0` |
27+
| Load more button | `fc-middle` | `pagination === "loadMore"` **and** `hasMoreItems` |
28+
| pagination bar | `*-end` | `paginationVisible` — see `Pagination.viewModel.ts` |
29+
| custom pagination | `fc-end` | `useCustomPagination`; suppresses the built-in bar, since `paginationVisible` returns `false` for `paginationKind === "custom"` |
30+
31+
## Goals / Non-Goals
32+
33+
Goals
34+
35+
- Left / Center / Right alignment that is actually where it claims to be, in both bars.
36+
- Visual order and focus order stay in agreement.
37+
- One placement rule shared by runtime and editor preview so they cannot drift.
38+
- Existing app configurations keep working without migration.
39+
40+
Non-Goals
41+
42+
- Pagination alignment for DataGrid 2 (no such property exists there; adding one is a feature).
43+
- Fixing DataGrid 2's custom-pagination position bug, or its `-padding-top` container-query typo (own branch and PR — DataGrid 2 changes are kept out of a Gallery PR).
44+
- E2E coverage for design properties (no test project currently loads the module CSS for Gallery).
45+
- Replacing the design property with an XML widget property (cleaner long-term, but that makes WC-3505 a feature rather than a fix).
46+
47+
## Decisions
48+
49+
### Displacement, not wrapping
50+
51+
Pagination claims the zone its alignment names; whatever was there moves to the end zone.
52+
53+
```
54+
align = Left, counter visible, buttons mode
55+
┌─────────────────┬─────────────────┬─────────────────┐
56+
│ [1-10 of 42 ◀▶] │ │ 3 selected │
57+
└─────────────────┴─────────────────┴─────────────────┘
58+
59+
align = Center, loadMore + total count + selection
60+
┌─────────────────┬─────────────────┬─────────────────┐
61+
│ 3 selected │ 1-10 of 42 │ [ Load more ] │
62+
└─────────────────┴─────────────────┴─────────────────┘
63+
```
64+
65+
The rule is total: at most three occupants exist, there are three zones, and custom pagination replaces the built-in bar rather than adding to it. Since pagination claims exactly one zone, at most one occupant is ever displaced, so the end zone never has to hold two things.
66+
67+
The alternative considered was wrapping the bar onto a second full-width row when the claimed zone is occupied. Rejected: the selection counter appears dynamically at `selectedCount > 0`, so wrapping would add a row — and shift the page — the moment a user selects their first item. Displacement keeps the bar one row tall at all times.
68+
69+
### Placement computed by a pure function, not expressed in CSS
70+
71+
Four mechanisms were considered:
72+
73+
| | TSX placement | CSS `order` | CSS grid areas | `margin: auto` |
74+
| --------------------------------- | ----------------- | --------------- | -------------- | -------------- |
75+
| DOM order matches visual | yes | no | no | no |
76+
| Focus / reading order correct | yes | no | no | no |
77+
| Needs to read the alignment value | yes | no | no | no |
78+
| Release vehicle | widget + module | module only | module only | module only |
79+
| Top bar Center | needs `tb-middle` | geometry rework | workable | fiddly |
80+
81+
The CSS mechanisms are cheaper — module-only release, no widget bump, no need to read the alignment at all — but every one of them reorders visually while leaving DOM order fixed. For a paging control that is a WCAG 2.4.3 (Focus Order) and 1.3.2 (Meaningful Sequence) defect: keyboard focus would jump right, then left, across the footer. TSX placement is chosen for that reason, and it also leaves the existing `< 500px` container queries untouched, since no new high-specificity selectors compete with them.
82+
83+
Placement logic is a pure function rather than inline JSX conditionals:
84+
85+
```
86+
resolveZones({ alignment, hasCounter, hasLoadMore, hasPagination })
87+
→ { start: "pagination" | "counter" | null,
88+
middle: "pagination" | "loadMore" | null,
89+
end: "pagination" | "counter" | "loadMore" | null }
90+
```
91+
92+
Algorithm: map alignment to a target zone; if pagination is visible, it takes that zone; then place each remaining occupant in its natural home (counter → start, Load more → middle) or, if that home is taken, in the end zone.
93+
94+
This keeps every alignment × occupancy combination testable without rendering, and lets the footer, the top bar and `Gallery.editorPreview.tsx` consume one shared result — the divergence that produced the custom-pagination bug below came precisely from those three places each deciding placement for themselves.
95+
96+
### Alignment is parsed from the design-property class
97+
98+
Design-property selections arrive as class names on the widget root. `props.class` is already piped through the props gate and surfaced by `GalleryRootViewModel.className`, so a MobX computed can parse it for `widget-gallery-pagination-(left|center|right)` and default to `right`. Because it is a computed over a string, Design-mode edits are reflected live and the parser is unit-testable on its own.
99+
100+
Trade-off accepted: three class names in `data-widgets`' `design-properties.json` become an input to `gallery-web`'s render logic. Renaming them would silently break layout. Mitigation is to treat them as a documented contract, asserted by unit tests on both sides of the parse.
101+
102+
The alternative — a new `pagingAlignment` XML enum — is better long-term design: typed, discoverable in the properties pane, no cross-package coupling. It was rejected for this change because it converts a bug fix into a feature, needs the existing design property deprecated with a migration story, and drops the Atlas-style ToggleButtonGroup affordance.
103+
104+
### Top bar gains a middle zone
105+
106+
Center is not expressible in a two-zone bar, so `widget-gallery-tb-middle` is added. It also makes the two bars structurally symmetric, so `resolveZones` applies unchanged to both (the top bar simply never has a Load-more occupant).
107+
108+
### `Both` + custom pagination renders once, with an editor warning
109+
110+
Custom pagination is a `widgets` placeholder holding real widget instances. Rendering it in both bars would duplicate those instances, their DOM ids and their state, so `Both` renders once in the footer and `Gallery.editorConfig.ts` raises a `check()` warning explaining it. `Above grid` and `Below grid` are honoured exactly.
111+
112+
### Design property modernised rather than replaced
113+
114+
`Pagination` becomes a `ToggleButtonGroup` with `Atlas_Core.Atlas.align-left` / `align-center` / `align-right` icons — the form Atlas Core uses for every other alignment control — and gains an explicit `Right` option instead of relying on the implicit unset entry.
115+
116+
The property keeps its name, and so do the `Left` and `Center` options. Studio Pro stores a design property selection by **property name and option name**, not by CSS class, so renaming any of them orphans every existing selection. Verified in Studio Pro: renaming the property to "Pagination alignment" raised two errors on a page that already used it — `CE6083` "Design property Pagination is not supported by your theme" and `CE6087` "Design properties have been renamed in your theme and need to be updated". `oldNames` is honoured (Studio Pro offers "Update all renamed design properties in project"), but that is a migration the app developer has to run, and the app carries errors until they do. A bug-fix release should not impose that on every consumer, so the clearer label is left for a future deliberate revision of this property.
117+
118+
This is also why the class names cannot be renamed: they are the stored values, and they are simultaneously the contract the widget parses. Both halves of the property — names and classes — are frozen.
119+
120+
## Risks / Trade-offs
121+
122+
- **Focus order now varies with a styling-looking property.** `Left` places the paging controls before the Clear-selection button in the tab sequence. Accepted deliberately: the alternative is visual and focus order disagreeing.
123+
- **Snapshot churn.** The new `tb-middle` node and zone reassignment change rendered DOM; component snapshots need regenerating and reviewing rather than blindly updating.
124+
- **Cross-package class contract.** Covered above; mitigated by tests and documentation.
125+
- **Centring depends on zone symmetry.** `fc-middle` is currently absent from the flex-sizing rules, so its centring is incidental — a counter long enough to hit min-content width skews it. The change gives the middle zones explicit sizing so Center holds by construction.
126+
- **No automated regression guard for the CSS half.** Unit tests cover the placement map and the class parser; the rendered alignment itself is verified by manual Studio Pro QA. See the proposal for why E2E is impractical here.
127+
128+
## Open Questions
129+
130+
- ~~Whether the regenerated `tests/testProject/themesource/datawidgets/**` copy is committed alongside the source SCSS edit~~ — resolved: left to the module build. No previous commit touching `_gallery.scss` has updated that copy.
131+
- Whether DataGrid 2's custom-pagination position bug is filed now or after this change lands.

0 commit comments

Comments
 (0)