test(ui): die drei Breiten-Zusicherungen der Küchen-Listen absichern - #643
Conversation
Die beiden CSS-Fixes aus PR #614 trugen ihre Begründung bisher nur als Kommentar in kitchen-row.css. Drei Invarianten hängen daran: 1. .kitchen-list darf kein max-width tragen. Es ist das Element mit overflow-y: auto; kappt man es, endet auch sein Trefferbereich fürs Mausrad an der Lesespalten-Kante. 2. .kitchen-group und .kitchen-rows tragen die Kappung stattdessen, weil jedes von beiden alleiniges Kind des Scrollers sein kann. 3. .kitchen-rows braucht align-self: start, sonst streckt es sich als Grid-Item auf die Spurhöhe und schneidet über sein overflow: hidden alles ab, bevor der Scroller den Überlauf sieht (gemessen an 50 gespiegelten Rezepten: scrollHeight 3249px gegen clientHeight 657px). Die dritte Zusicherung ist bewusst eine Regel und keine Allowlist: wer aufs Lesemaß kappt UND clippt, ist ein gekapptes Kind des Scroller-Grids und muss align-self: start tragen. Damit fällt auch eine künftige Gruppe darunter, die unter anderem Namen dieselbe Kombination baut.
There was a problem hiding this comment.
Pull request overview
This PR strengthens the existing frontend audit suite by adding regression guards for three CSS layout invariants in the shared kitchen list styles introduced/justified in PR #614, without changing production CSS.
Changes:
- Adds assertions that
.kitchen-listmust not setmax-width, preserving full-width scroll hit-testing on wide viewports. - Adds assertions that
.kitchen-groupand.kitchen-rowsmust carrymax-width: var(--content-max-width-narrow). - Adds a rule-based scan ensuring any rule that both caps to the narrow content width and clips with
overflow: hiddenalso hasalign-self: start.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce6566501b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review-Nachlauf zu #643. Der Guard prüfte nur den ersten Textblock je Selektor und nur kitchen-row.css. Drei Wege führten daran vorbei: - Ein zweiter `.kitchen-list`-Block hinter einem Breakpoint. `cssRuleBody` liefert den ersten Treffer, der zweite kappte den Scroller unbemerkt. - Das Modul-CSS. Der Scroller trägt seine Modul-Klasse auf demselben Element (`class="kitchen-list items-list"`), also kappt ihn auch ein `max-width` in recipes/pantry/shopping.css, und ein `align-self` ebendort biegt den Stretch-Fix wieder um. - Ein Custom Property. Die Muster waren nicht an der Deklarationsgrenze verankert, deshalb erfüllte `--x: var(--content-max-width-narrow)` die geforderte Kappung und `--y-max-width:` löste einen Fehlalarm aus. Gescannt wird jetzt jede Regel jeder Stylesheet-Datei. Welche Klassen den Scroller mitbenennen, liest der Guard aus dem Markup statt aus einer Liste. Die Mechanik-Regel für align-self bleibt auf kitchen-row.css: andere Module kappen mit demselben Token Elemente, die nie Grid-Item dieses Scrollers werden.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d5a7ad85f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Zweiter Review-Nachlauf zu #643. Vier Wege führten noch daran vorbei: - `.kitchen-list#items-list` und `:is(.kitchen-list)`. Der Selektor-Test verlangte, dass die Klasse den Selektor beendet, und ließ ein ID-Suffix nicht zu. Geprüft wird jetzt der letzte Compound, `:not(…)` vorher entfernt, damit ausgerechnet der Ausschluss keinen Treffer meldet. - `place-self: stretch`. Die Kurzschreibweise setzt align-self mit zurück, der Override-Schutz suchte aber nur die Langform. - `max-width: var(--content-max-width-narrow); max-width: none` in einer Regel. Geprüft wurde, OB der geforderte Wert vorkommt, nicht welcher gewinnt. Jetzt zählt die letzte Deklaration, wie im Browser. - Eine Seite, die aus der Klassen-Erkennung fällt. Die globale Mindestzahl hätten die beiden anderen weiter erfüllt, während die Modul-Klasse der ausgefallenen ungeprüft bliebe. Jede Seite muss jetzt ihre eigene Kombination liefern. Statt `declares(body, prop, wert)` gibt `declaredValue(body, props)` den wirksamen Wert zurück: letzte Deklaration, Kurzschreibweisen eingerechnet.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e00961a88a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
test/test-frontend-audit.js:1979
- Der Guard für Invariante 1 prüft nur Selektoren, die über Klassen auf dem Scroller matchen (scrollerClasses + rulesFor(cls)). Ein
max-widthkann aber genauso per ID-Selektor (z.B.#items-list,#pantry-list,#recipes-list) auf dasselbe.kitchen-list-Element gelegt werden und würde dann unbemerkt bleiben, obwohl das die Invariante genauso bricht. Da die Scroller-Elemente in den Seiten jeweils IDs tragen, sollte der Scan diese IDs ebenfalls aus dem Markup extrahieren und alle Regeln prüfen, die die ID im letzten Compound targeten.
const scrollerClasses = new Set(['kitchen-list']);
for (const page of ['shopping', 'pantry', 'recipes']) {
const combos = [...read(`../public/pages/${page}.js`)
.matchAll(/class(?:Name)?\s*=\s*(['"`])([^'"`]*\bkitchen-list\b[^'"`]*)\1/g)];
assert.ok(combos.length > 0,
Dritter Review-Nachlauf zu #643. Fünf weitere Wege führten daran vorbei: - `!important`. Die gewinnende Deklaration ist nicht immer die letzte: `max-width: none !important; max-width: var(…)` sah erfüllt aus. - `max-inline-size`, `inline-size`, `width`. Dieselbe Kappung, andere Schreibweise. Geprüft wird jetzt dasselbe Vierer-Paar, das der Modul-Root-Breiten-Guard schon verwendet. - `#recipes-list { max-width: … }`. Alle drei Scroller tragen eine ID; eine Regel darauf nennt keine ihrer Klassen. Die IDs kommen wie die Klassen aus dem Markup. - `--content-max-width-narrow` lokal umdefiniert. Das Kind trüge weiter die geforderte Deklaration, löste sie aber auf ein anderes Maß auf. Das Token hat jetzt genau eine Quelle: tokens.css. - `.page:has(.kitchen-list)`. Der umgekehrte Fall, ein Fehlalarm: die Regel nennt die Klasse, gestaltet aber den Vorfahren. `:has(…)` fällt wie `:not(…)` vor dem Zerlegen des Selektors weg. Nicht abgedeckt bleibt ein struktureller Selektor auf dasselbe Element (`.recipes-list > ul`). Die Menge solcher Selektoren ist nicht aufzählbar; sie zu erfassen hieße, Kaskade und Selektor-Matching nachzubauen, und dann misst man besser im Browser.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d266c91cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…weise Vierter Review-Nachlauf zu #643. Fünf Fälle, drei davon echte Lücken: - `width: 20rem` an einem gekappten Kind. Das max-width bleibt korrekt stehen, die Liste steht trotzdem schmal. Prozentwerte und `auto` bleiben erlaubt: sie messen den ungekappten Scroller, das max-width begrenzt weiter. - `overflow: clip` und `overflow-y: hidden`. Beide schneiden ab wie `hidden`; der Grund für die Zusicherung ist das Abschneiden, nicht die eine Schreibweise dafür. - `!IMPORTANT` in Großschreibung. CSS liest die Annotation case-insensitiv, die Erkennung tat es nicht. - Eine auf einen Selektor gescopte Neudefinition des Lesemaß-Tokens IN tokens.css. Ausgenommen ist jetzt die kanonische :root-Deklaration, nicht mehr die ganze Datei. Dazu ein Fehlalarm in der Gegenrichtung: `.recipes-list::before { width: 1rem }` galt als Kappung des Scrollers. Ein Pseudo-Element ist ein eigener Kasten, nicht das Element - solche Selektoren fallen jetzt raus.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64426f7114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ibweisen schliessen Fünfter Review-Nachlauf zu #643. Sechs Wege blieben: - `MAX-WIDTH: 20rem`. Eigenschaftsnamen sind ASCII-case-insensitiv, die Erkennung war es nicht. Custom Properties bleiben case-sensitiv, dort sind `--Foo` und `--foo` zwei Namen. - `width: 50%` an einem gekappten Kind. Der Freibrief für Prozentwerte war zu weit; unschädlich ist nur die volle Breite. - `align-self: start` am SCROLLER. Der ist Flex-Item seines Modul-Roots; ohne das voreingestellte Strecken schrumpft er auf Inhaltsbreite, und der Mausrad-Trefferbereich endet dort - ganz ohne Breitenangabe. - `list.style.maxWidth = '20rem'`. Inline-Stile stehen in keiner gescannten Datei und schlagen trotzdem jede Regel darin. Der Scan sucht sie jetzt im Seiten-JS, an der Variablen, die die Klasse bekommt. - Die gelöschte :root-Deklaration. Ohne sie löst var(…) auf nichts auf, max-width fällt auf `none`, und der Textvergleich stimmt weiterhin. - Beide Kind-Kappungen hinter einem Breakpoint. `scopedRules()` merkt sich jetzt, ob eine Regel in einer At-Rule steht; die geforderte Kappung muss unbedingt gelten.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 534d3d31a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…schliessen Sechster Review-Nachlauf zu #643. Der erste Punkt ist ein Fehler in dem Parser, den der vorige Commit eingefuehrt hat: - `scopedRules()` behandelte `@import url(…);` wie einen Blockanfang. Das Praeludium wuchs ueber die Anweisung hinaus, begann mit '@' und die erste echte Regel der Datei wurde still als At-Rule-Rumpf verschluckt. budget.css nutzt `@import` bereits; der Guard haette dort weniger geprueft, als er behauptet. Semikola ausserhalb von Regelbloecken beenden das Praeludium jetzt. Dazu vier Umgehungen und ein zweiter Weg zum Inline-Stil: - `.kitchen-rows:hover` als einzige Kappung. `conditional` kannte nur At-Rules; die geforderte Deklaration muss auch aus einem Selektor ohne Zustand kommen, sonst laeuft die Liste ueberall ausser unter dem Mauszeiger voll breit. - `justify-self: start` am Kind. Als Grid-Item fuellt es seine Spur per Voreinstellung; ohne die faellt die auto-Breite auf den Inhalt zusammen, waehrend das Lesemass unangetastet dasteht. Der zweite Teilwert von `place-self` setzt dasselbe. - `all: initial`. Setzt Kappung und Ausrichtung zurueck, ohne eine der geprueften Eigenschaften zu nennen. - `setAttribute('style', …)` statt `.style.maxWidth`. Nicht abgedeckt bleibt die Ausrichtung am Elternteil (`.recipes-page { align-items: start }`). Dort beginnt die Vorfahren-Kette, und die ist ohne DOM nicht abschliessend pruefbar - dieselbe Grenze wie beim strukturellen Selektor.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f10d307d42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // weiter zwei Texte vergleicht, die zueinander passen. | ||
| assert.ok(allRules.some(({ file, selectors, body, conditional }) => | ||
| file === 'tokens.css' && !conditional && selectors.some((sel) => /^:root\b/.test(sel)) | ||
| && declaredValue(body, '--content-max-width-narrow') !== null), |
There was a problem hiding this comment.
This existence check accepts any declared value, so changing the canonical declaration to --content-max-width-narrow: none leaves the frontend audit green while both child max-width declarations resolve to none and no longer cap the lists. Validate that the token contains a usable narrow length rather than merely checking that the property was parsed.
Useful? React with 👍 / 👎.
| if (prelude.startsWith('@')) { | ||
| depth += 1; | ||
| from = i + 1; |
There was a problem hiding this comment.
When the canonical .kitchen-group or .kitchen-rows rule is moved into an unconditional @layer components { ... }, this parser increments depth exactly as it does for @media, marks the rule conditional, and fails the required-cap assertion even though cascade layers apply at every viewport and state. Classify block at-rules by semantics instead of treating every @ prelude as conditional.
Useful? React with 👍 / 👎.
| assert.equal(declaredValue(body, WIDTH_CAP), null, | ||
| `${file} ${selectors.join(', ')}: der Scroller darf nicht gekappt werden, sonst endet sein Mausrad-Trefferbereich an der Lesespalten-Kante`); |
There was a problem hiding this comment.
This assertion rejects every size declaration regardless of its value, so a harmless module rule such as .recipes-list { max-width: none; } or width: auto fails the audit even though it explicitly leaves the scroller unconstrained and preserves the full wheel-hit area. Check whether the effective value actually caps the size rather than requiring the properties to be absent.
Useful? React with 👍 / 👎.
| assert.ok(rules.some(({ body, conditional, selectors }) => | ||
| !conditional && selectors.some(unstated) && declaredValue(body, MAX_WIDTH) === NARROW), |
There was a problem hiding this comment.
This existence check only proves that an unconditional selector containing the shared class declares the token; it does not prove that any real child matches the rest of that selector. Replacing the base rule with .shopping-page .kitchen-rows { max-width: var(--content-max-width-narrow); } leaves the audit green even though the directly rendered recipe rows no longer match any cap and become full width. Validate the required cap for each rendered child context rather than accepting any selector returned by rulesFor.
Useful? React with 👍 / 👎.
| if (declaredValue(body, MAX_WIDTH) !== NARROW) continue; | ||
| // `clip` kappt wie `hidden`, nur ohne Scrollport - und die Block-Achse | ||
| // lässt sich auch als Langform setzen. Der Grund für die Zusicherung ist | ||
| // das Abschneiden, nicht die eine Schreibweise dafür. | ||
| const overflow = declaredValue(body, ['overflow', 'overflow-y', 'overflow-block']); |
There was a problem hiding this comment.
The clipping invariant is evaluated within each individual rule body, although CSS combines declarations from every matching rule. Splitting the current styles into .kitchen-rows { max-width: var(--content-max-width-narrow); } and a separate .kitchen-rows { overflow: hidden; }, while removing align-self, leaves the audit green even though the computed element is still capped and vertically clips its stretched content. Evaluate the effective combination across matching rules instead of requiring the cap and overflow to occur in the same block.
Useful? React with 👍 / 👎.
| // Eine Kappung ist eine Kappung, egal wie buchstabiert: die logischen Formen | ||
| // wirken im Schreibmodus dieser App auf dieselbe Achse. Dasselbe Paar prüft | ||
| // der Modul-Root-Breiten-Guard weiter unten schon. | ||
| const WIDTH_CAP = ['width', 'max-width', 'inline-size', 'max-inline-size']; |
There was a problem hiding this comment.
The audited size and alignment properties are not the only way to stop this column-flex item from filling its cross axis. Adding .recipes-list { margin-inline-end: 20rem; } leaves the frontend audit green, but stretch sizing subtracts that margin from the scroller's border box, so its wheel-hit area again ends 20rem before the available page edge. Include nonzero inline margins in the scroller-width guard or verify the computed hit area.
Useful? React with 👍 / 👎.
| const end = live.indexOf('}', i); | ||
| if (end === -1) break; |
There was a problem hiding this comment.
This flat scanner treats the first closing brace after a selector as the end of its rule, so valid native nesting such as .recipes-list { & { max-width: 20rem; } } is recorded only as an outer body containing no recognizable declaration and the nested rule is never emitted. I added that rule to the later-loaded recipes stylesheet and the audit remained green even though the nested selector caps the real scroller. Traverse balanced nested blocks or reject nesting explicitly instead of silently skipping their declarations.
Useful? React with 👍 / 👎.
…#644) Nachlauf zu #643. Vier Review-Runden, 23 Befunde, alle nachvollzogen und umgesetzt. Die wichtigsten waren die Fehlalarme - Fälle, in denen der Guard korrekte Arbeit blockiert hätte: - `max-width: none` und `width: auto` am Scroller galten als Kappung. - Eine Kappung in `@layer components` galt als bedingt. - `margin-inline-end: 20rem; margin: 0` wurde als Verstoß gemeldet, obwohl der Shorthand die Marge auf null setzt. - `align-self: revert` wurde abgelehnt, weil die eingeführte FILLS-Liste nirgends benutzt wurde. Dazu die Lücken: natives CSS-Nesting und At-Rules innerhalb von Style-Regeln (beide im Scanner), `@starting-style`, `--content-max-width- narrow: none` und `720px bogus`, !important zwischen Token-Regeln und beim Kaskadieren der Margen, `width` neben `max-width`, `:is()`-Inhalte, `:not()`/`:has()` als Bedingung, Selektor-Gruppierung nach Element statt Text, Vorfahren-Kontext und Zustand beim Zusammenfassen, setProperty und Inline-Margen im JS. Der Leitgedanke durch alle Runden: nicht die Schreibweise prüfen, sondern die Wirkung. Mehrfach war genau das die Ursache - eine Prüfung fragte, ob eine Eigenschaft dasteht, statt was sie bewirkt. 77 Mutationen belegen den Stand, davon 15, die grün bleiben müssen. npm test vollständig: 2660 grün. Kein CSS geändert. Bekannte Grenzen, beide brauchen Kaskade oder DOM: ein struktureller Selektor auf ein geschütztes Element und eine Ausrichtung, die von einem Vorfahren kommt.
PR #614 brachte zwei CSS-Fixes in
public/styles/kitchen-row.cssmit, für die kein Guard existiert. Ihre Begründung steht bis jetzt ausschließlich als Kommentar im CSS. Der FAB-Bereich daneben ist dreifach abgesichert, diese Zeilen gar nicht.Kein CSS geändert: die Zeilen sind korrekt, sie waren nur ungeschützt.
Die drei Invarianten
.kitchen-listdarf keinmax-widthtragen. Es ist das Element mitoverflow-y: auto. Kappt man es aufs Lesemaß, endet damit auch sein eigener Trefferbereich fürs Mausrad an der Lesespalten-Kante, und auf einem breiten Fenster greift das Rad rechts davon ins Leere..kitchen-groupund.kitchen-rowstragen die Kappung stattdessen. Jedes von beiden kann alleiniges Kind des Scrollers sein:.kitchen-groupbei gruppierten Tabs (Einkauf, Vorrat),.kitchen-rowsungruppiert (Rezepte). Fehlt sie an einem der beiden, läuft der betroffene Tab bildschirmbreit..kitchen-rowsbrauchtalign-self: start. Als Grid-Item von.kitchen-liststreckt es das voreingestelltealign-items: stretchsonst auf die volle Spurhöhe, und es schneidet über seinoverflow: hidden(für die Eckenradien) alles darüber still ab, bevor der eigentliche Scroller den Überlauf je sieht. Gemessen an einer Rezeptliste mit 50 gespiegelten Einträgen: scrollHeight 3249px gegen clientHeight 657px, kein Scrollbalken.Regel statt Allowlist
Zusicherung 3 nennt
.kitchen-rowsnicht. Sie läuft über alle Regeln der Datei und verlangtalign-self: startvon jedem Selektor, dermax-width: var(--content-max-width-narrow)UNDoverflow: hiddenträgt: wer aufs Lesemaß kappt und zugleich clippt, ist per Definition ein gekapptes Kind des Scroller-Grids. Damit fällt auch eine künftige Gruppe darunter, die unter anderem Namen dieselbe Kombination baut.Das Lesemaß ist als Kriterium nötig statt irgendeines
max-width:.item-tagin derselben Datei kombiniertmax-width: 12remmitoverflow: hiddenund ist ein Zeilen-Etikett, kein Kind des Scrollers.Verifikation
Jede Zusicherung einzeln fallen gelassen und den roten Lauf bestätigt:
max-widthzurück an.kitchen-listmax-widthaus.kitchen-groupentferntmax-widthaus.kitchen-rowsentferntalign-self: startentferntInvariante 1 ist eine Abwesenheit und lässt sich nur durch Wiedereinsetzen der Kappung auslösen; genau so geprüft. Zusätzlich hat ein frei erfundener Selektor mit derselben Kombination den Test rot gemacht, obwohl sein Name im Guard nirgends steht - das ist der Unterschied zur Allowlist.
npm testvollständig: 2660 grün, 0 rot.