Repository navigation
spec(i18n, Resizable, AST-103): factories take announcement and label options with a catalog default - #9395
Conversation
…s with a catalog default
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
|
/approve-spec a72e5e1 |
There was a problem hiding this comment.
The per-message option rule is clear and owner-approved at this head, but two record fixes are needed so the current records agree.
architecture:internationalizationINV3a lets a caller replacecreateReorderable's announcements, whilespec:AST-103FR1, FR4b and FR4b's verification row still allow only shipped-English strings and three options. Either amend AST-103 here, saying how an option replaces a message with arguments such as "position 3 of 5", or limit INV3a to INV22 factories.component:ResizableAR1 describescollapsedLabelonCreateResizableOptionsanduseResizable(handleRef, options), which do not exist on main yet. Add an implementation gap and a Current-state impact bullet, as the record does for its other unbuilt clauses.
Optionally, word INV3a's scope as "writes or announces Astryx text (live-region announcements and aria-*/title values)" so it plainly covers aria-valuetext. After the next push, the new head needs a fresh /approve-spec.
[Automated review]
| - **INV3a — Factories take a per-message option for each announced string.** A | ||
| public non-React factory (`architecture:public-component-api` INV22) that | ||
| announces Astryx text takes an optional string option for each announced | ||
| message; given, it replaces that message; absent, the factory announces the | ||
| catalog message, translated when its React wrapper runs under a provider and | ||
| English without one. Per-message options are the factory mechanism; the | ||
| external-runtime adapter (INV14) stays a separate, broader decision. |
There was a problem hiding this comment.
spec:AST-103 FR1 lists createReorderable's options as onReorder, isReorderable and positions. Its FR4b has attach use only shipped-English strings, and FR4b's verification row fails on "strings other than the shipped English catalog's". If INV3a covers that factory, as the description says, AST-103 needs the matching amendment in this PR. INV3a should also say how an option replaces a message with arguments, such as "Milk, position 3 of 5, after Eggs, before Bread": for example, an ICU message taking the catalog message's arguments. Otherwise, limit INV3a to the create…(element, options) factories of INV22.
| `en`, direction `ltr`, and do not read the browser or host locale implicitly. | ||
| - **INV3a — Factories take a per-message option for each announced string.** A | ||
| public non-React factory (`architecture:public-component-api` INV22) that | ||
| announces Astryx text takes an optional string option for each announced |
There was a problem hiding this comment.
Minor: INV1a separates "live-region announcements" from "aria-*/title values", but here the scope is only text a factory "announces", and Resizable AR1 applies it to aria-valuetext. Suggest: "that writes or announces Astryx text (live-region announcements and aria-*/title values)".
| `CreateResizableOptions`, which `useResizable(handleRef, options)` also | ||
| takes, has `collapsedLabel?: string`, announced as the Handle's | ||
| `aria-valuetext` while collapsed; absent, the catalog's "Collapsed" | ||
| (`architecture:internationalization` INV3a). |
There was a problem hiding this comment.
Neither CreateResizableOptions nor useResizable(handleRef, options) exists on main: useResizable.ts exports only useResizable(config). This record's summary says "Implementation gaps are listed under Current-state impact", and its compatibility line says "No prop, option, or return member changes". Suggest appending "Implementation gap: no createResizable or ref-taking useResizable exists yet, so no caller can pass collapsedLabel (Current-state impact)" with the matching bullet.
…edLabel as not built
|
/approve-spec cd28713 |
There was a problem hiding this comment.
The three record fixes from the last review are in and the records now agree; one description edit remains. INV3a now covers only INV22 create…(element, options) factories and leaves spec:AST-103's shipped-English strings in place, but the description still says "INV3a covers it, and its implementation adopts the rule" about createReorderable, and that any non-React factory takes per-message options. Please narrow the Maintainer impact, Existing authority searched and implementation lines to INV22 factories, and note that adopting the rule for createReorderable also amends AST-103 FR1 and FR4b. A description edit makes no new commit, so the current /approve-spec still applies.
[Automated review]
| English without one. The rule covers INV22 factories; a gesture or | ||
| interaction spec that fixes its own announcement options keeps them until it | ||
| adopts this rule. Per-message options are the factory mechanism; the |
There was a problem hiding this comment.
This sentence keeps createReorderable out of INV3a for now: createReorderable(options) is not an INV22 create…(element, options) factory, and spec:AST-103 FR4b keeps its shipped-English strings. The PR description still says "INV3a covers it, and its implementation adopts the rule" and speaks of any non-React factory; please narrow those lines to INV22 factories and note that adopting the rule there also amends AST-103 FR1 and FR4b.
There was a problem hiding this comment.
The record changes are owner-approved at this head and agree with the current specs, and the description now matches INV3a's INV22-only scope, so this review has nothing further to ask. collapsedLabel and INV3a's factory tests land with the createResizable implementation.
[Automated review]
PR Analysis Report🔎 Preview ·
|
…r several; createReorderable takes labels
|
/approve-spec a174e10 |
There was a problem hiding this comment.
The record changes are owner-approved at this head, but two of them disagree with each other or with the current AST-103 text and need a record edit before merge.
- INV3a cites
createResizable's flatcollapsedLabelas its one-string example, yet the Resizable handle also shows the catalog name "Resize handle", so by INV3a's own grouping rule and verification row it is a multi-string factory: either move Resizable tolabels: {handle, collapsed}, or say in INV3a that an existing name option such aslabelstays flat and is not counted. ReorderableLabelshas no key for FR4's tree move announcement ("a tree adds the parent"), although FR1 sayslabelsholds every string the object announces: add a tree key such asplaceInTree, or state that tree announcements stay outsidelabels.
A new commit needs a fresh /approve-spec for its head.
[Automated review]
| factory with one string takes a flat optional string option for it, such as | ||
| `createResizable`'s `collapsedLabel`; a factory with more than one groups | ||
| them under one optional `labels` object, each key optional. An absent option |
There was a problem hiding this comment.
The Resizable handle shows two Astryx strings: its name defaults to the catalog's "Resize handle" (ResizeHandle.tsx, @astryx.resizable.handle.label) beside "Collapsed". Under this sentence that makes createResizable a multi-string factory, and a flat collapsedLabel beside a flat label fails this record's INV3a verification row ("groups more than one string outside one labels object"). Either move Resizable to labels and change this example, or say that an existing accessible-name option such as label stays flat and is not counted.
| position), `place`, `placeAtTop`, `placeAtEnd`, and `placeInGrid` (FR4's | ||
| move announcements), `refused` (a move past a rejected row), `notMoved` |
There was a problem hiding this comment.
FR4 also announces a tree move with its parent ("a tree adds the parent"), and none of these four keys carries one, so a TreeList using this object would announce a string no labels key replaces, which FR1's verification row fails. Add a tree key (such as placeInTree, with the parent argument), or say tree move announcements stay outside labels.
…InTree for tree moves
|
/approve-spec 4657870 |
There was a problem hiding this comment.
The record changes are owner-approved at this head, and both record fixes from the last review are in: INV3a now keeps an existing accessible-name option such as label flat and uncounted, so createResizable's flat collapsedLabel is its one-string case, and ReorderableLabels gains placeInTree for FR4's tree move announcement. Nothing further is needed from this review; collapsedLabel, labels and their factory tests land with each factory's implementation. One optional tidy-up: the description still counts eleven announcement keys where FR1 now lists twelve.
[Automated review]
Maintainer impact
labelsobject. No translator or catalog key becomes part of a factory's public contract.createResizableanduseResizable(handleRef, options)takecollapsedLabel?: string, the Handle'saria-valuetextwhile collapsed.createReorderabletakeslabels?: ReorderableLabels, holding its eleven announcements and nine shown labels (handle name and hint, move buttons, Done).Exact decision needed
How a public non-React factory under
architecture:public-component-apiINV22 (create…(element, options)) lets callers localize the Astryx text it announces or shows when no provider is present.Decision
labelsgrouping for a factory with more than one string, and thatcreateReorderable'slabelsalso holds the labels it shows.Changed records
architecture:internationalization— new INV3a beside INV3: a public non-React factory (architecture:public-component-apiINV22) that announces or shows Astryx text (a live-region announcement, anaria-*ortitlevalue, or a visible label) takes an option for each such string: a flat optional option when it has one string (such ascreateResizable'scollapsedLabel), one optionallabelsobject with optional keys when it has more. An absent option or key falls back to the catalog message, translated under a provider and English without one; a given string replaces that message, including over a provider's translation. No factory takes a translator. The external-runtime adapter (INV14) stays a separate, broader decision. Added to the accessibility, behavior and public-api review routes, with a verification row (evidence gap: no factory test covers a flat option orlabelskey yet).architecture:public-component-apiINV22 — one clause: a factory's announced and shown strings follow internationalization INV3a's options.component:ResizableAR1 —CreateResizableOptions, whichuseResizable(handleRef, options)also takes, hascollapsedLabel?: string, announced as the Handle'saria-valuetextwhile collapsed; absent, the catalog's "Collapsed". AR1 is the record clause that decides the newCreateResizableOptions.collapsedLabel?: stringrow (architecture:public-component-apiINV25). Implementation gap: not built.spec:AST-103FR1 —createReorderable's options gainlabels?: ReorderableLabels. Announcement keys:pickedUp,dropped,cancelled,place,placeAtTop,placeAtEnd,placeInGrid,refused,notMoved,modeDone,unnamedItem. Shown keys:handle,handleVerb,unnamedHandle,handleHint,moveUp,moveDown,moveLeft,moveRight,done. Every key is optional and falls back to the catalog message; a templated key takes the catalog's ICU template with the same arguments (such as position and count). Implementation gap: not built.spec:AST-103FR4b —attachtakes each string fromlabels, an absent key falling back to the shipped English catalog, replacing its shipped-English-only strings. Implementation gap: not built.spec:AST-103verification — the FR1 row coversReorderableLabels' keys and fails on a string no key replaces or a translator option; the FR4b row covers every key given and none, and a given key under a provider (evidence gap: not built).spec:AST-103DEC-6 — onelabelsobject holds every string; rejected a translator option and one flat option per string.Existing authority searched
architecture:internationalizationINV1, INV1a, INV3, INV14;architecture:public-component-apiINV22, INV25;component:ResizableAR1;spec:AST-103FR1, FR4, FR4a, FR4b;spec:AST-107.aria-valuetext(@astryx.resizable.collapsed), not a live region; feat(Resizable): createResizable is public, and component:Resizable FR3 names it as the owner ResizeHandle and useResizable go through #9372 keeps it there and resolves it from the shipped English catalog in the bare factory.createReorderable's catalog messages in feat(core): createReorderable's positions() and attach() for grids, fans and plain elements (spec:AST-103) #8719 (@astryx.list.reorder*): eleven announcements and nine shown labels, whichReorderableLabelscovers one key each.Rejected alternatives
createReorderable.Claim scope and non-goals
collapsedLabelandlabelsland with each factory's implementation.Evidence and implementation relationship
createResizable) adoptscollapsedLabel; feat(core): createReorderable's positions() and attach() for grids, fans and plain elements (spec:AST-103) #8719 (createReorderable) adoptslabels.Writing rubric results
#digits,/pull/,/issues/in the added lines).Scope
Validation
pnpm install --frozen-lockfilepnpm check:knowledge— "Knowledge templates and records are aligned."git diff --check