Repository navigation
fix(Tokenizer): hold back the touch sheet presentation until its UX pass finishes - #8735
Conversation
…ass finishes The implementation stays, unexposed. `TokenizerProps` offers no `experimental.presentation`; every public Tokenizer renders the inline combobox. Astryx's own packages reach the sheet-capable signature as `TokenizerWithPresentation` through the internal entry `@astryxdesign/core/internal/tokenizer-presentation` (architecture:react-component-runtime INV13), and the sheet tests run through it. The experimental prop and example leave `Tokenizer.doc.mjs` and the experimental stories leave Storybook; the pending [experimental] changeset is replaced by an internal [chore] one; the Tokenizer record carries the implementation gap on DEC-3, FR16–FR24, Public concepts and readiness.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
|
/approve-spec 5861a7a |
PR Analysis Report
|
There was a problem hiding this comment.
Requesting changes: the hold works in types, docs and stories, but the public Tokenizer is still the same function as TokenizerWithPresentation and reads experimental.presentation at runtime, so a JavaScript caller or a JSX spread (which type-checks) still opens the unfinished sheet. Please put the option behind a non-exported boundary, for example by having the public export drop experimental before it renders a shared internal implementation, and add a test that the public barrel's Tokenizer given a spread experimental still renders the combobox; that also makes the record's and changeset's "every field renders the popover presentation" true. In the same pass, the INV7 exception in Tokenizer.spec.md still names the deleted Experimental: adaptive story as its measurement fixture, so it needs a fixture that still exists.
[Automated review]
| * DEC-3). The public `Tokenizer` renders the popover presentation for every | ||
| * field. | ||
| */ | ||
| export const TokenizerWithPresentation = Tokenizer as < |
There was a problem hiding this comment.
TokenizerWithPresentation is this same function, so the public Tokenizer still reads experimental.presentation at runtime (L754, L780): a JSX spread such as {...{experimental: {presentation: 'bottom-sheet' as const}}} type-checks against the public signature and opens the sheet. To make the hold real, the public export could drop experimental before rendering a shared, non-exported implementation that TokenizerWithPresentation renders with the option, plus a test that the public barrel's Tokenizer given a spread experimental renders the combobox.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: Tokenizer's touch sheet presentation is held back until its user experience is finished. The implementation stays in the package, reachable only through an internal entry reserved for Astryx's own packages; `TokenizerProps` offers no `experimental.presentation`, every Tokenizer renders the inline combobox, and nothing a consumer sees changes. |
There was a problem hiding this comment.
"Reachable only through an internal entry" and "every Tokenizer renders the inline combobox" hold only for callers that pass the attribute directly in TypeScript; a spread or a JavaScript caller still reaches the sheet through the public export (see the comment on Tokenizer.tsx).
…/c09-token-remove-keyboard
… its internal entry) into meta#8105 useAnnounceRenderedText.adopters.test.tsx: main's test now reaches the sheet through TokenizerWithPresentation (the internal entry). The sheet still loads lazily, and with this branch its chunk carries the release wiring, so the sheet case's 1 s findByLabelText still measures a cold import: main's file alone fails the case 2 of 2 here. The untimed preload of TokenizerBottomSheet stays, after main's import: 3 of 3.
Maintainer impact
Tokenizer's touch sheet presentation (
component:TokenizerDEC-3, FR16–FR24) is onmainbehindexperimental={{presentation}}, and the next release would announce it. Its user experience is still being finished on the Lab components, so a consumer who opts in today gets a surface whose details are about to move. The touch sheet is held back until its UX pass finishes; the implementation stays, unexposed.Intended invariant
spec:AST-017FR21: an internal entry is undocumented, outside the public API and its compatibility promise, versioned in lockstep with Core.architecture:react-component-runtimeINV13: a surface Astryx's own packages need but consumers must not see is reached through@astryxdesign/core/internal/*, a subpath limited by lint (@astryx/no-core-internal-import) to Core and Lab.component:TokenizerFR1–FR15, AR1–AR5 (the inline combobox) are unchanged: every public Tokenizer renders the popover presentation, as before feat(core): Tokenizer touch sheet behind experimental.presentation #7977.docs/contributing/spec-writing.md.Change
Public API.
TokenizerPropsno longer hasexperimental; the publicTokenizerhas one signature,TokenizerProps, soComponentProps<typeof Tokenizer>and every public call name no presentation and keep the released input-onlyref. The implementation is untouched:Tokenizer.tsxstill reads the presentation options and rendersTokenizerBottomSheet(lazy) in the sheet branch.Internal path: the INV13
./internal/*entry, not an internal-only prop.@astryxdesign/core/internal/tokenizer-presentationre-exportsTokenizerWithPresentation— the sameTokenizerfunction typed withTokenizerWithPresentationProps(experimental?: {presentation}andref: Ref<HTMLInputElement | HTMLButtonElement>) — plus the two types, by name. Why this and not a__-prefixed prop on the public type: the repo already has the entry pattern for exactly this case (internal/touch, #8464): the subpath is registered inpackage.jsonexportsandscripts/sync-exports.js, is undocumented, is outside the compatibility promise, and lint refuses it anywhere but Core and Lab — so the sheet is reachable by MobilePowerSearch (#8001, on hold) and by Core's own tests, and by nothing else. An internal prop omitted from the public type would still ride the public component's props at runtime and would need its own convention; an entry costs oneexportand a cast, no runtime. The sheet suite (TokenizerBottomSheet.test.tsx, 62 tests), the SSR test and the own-content adopters test now renderTokenizerWithPresentationfrom../internal/tokenizer-presentation;Tokenizer.props-type.test.tsxproves, with@ts-expect-error, thatexperimentalis not onTokenizerPropsnor on the public component, that the publicrefstays input-only, and that onlyTokenizerWithPresentationadmits the option and the wider ref. A newinternal/tokenizer-presentation.test.tschecks the entry re-exports the one component by name and that the publicTokenizerbarrel exports exactlyTokenizer.experimental-isolation(CLI goldens) agrees: nothing public names the sheet.Docs and stories.
Tokenizer.doc.mjsis back to its content before #7977 (theexperimentalprop entry in the English and Chinese prop tables and the dense summary are gone — that commit was the only change to the file since, so the restore is exact; 16 lines). The three experimental stories (Experimental: bottom sheet,Experimental: adaptive,Experimental: bottom sheet states) leaveTokenizer.stories.tsx(140 lines, likewise the file's only change since). They are not kept under an internal location: Storybook has none — everystories/**/*.stories.*is published to the docsite's/storybook/— and the Storybook app may not import the internal entry (the lint rule limits it to Core and Lab), so a dev-only story could not reach the sheet. The sheet's browser evidence remains in #7977's description and its jsdom coverage in the sheet suite.Changeset. #7977's pending
[experimental]changeset (tokenizer-experimental-bottom-sheet.md) is removed so the release notes do not announce the sheet.check:changesetsrequires a categorized changeset for a@astryxdesign/corechange; following the repo's internal-change convention ([chore] Internal: … Nothing a consumer sees changes., as incore-hit-area.mdandanimated-layout.md),tokenizer-sheet-presentation-held-back.mdsays the sheet is held back, reachable only through an internal entry, and that nothing a consumer sees changes.Record (
Tokenizer.spec.md). A present-tense implementation gap on DEC-3 and on each of FR16–FR24 ("Implementation gap: the sheet presentation is implemented but not yet exposed;TokenizerPropsoffers noexperimental.presentationwhile its user experience is being finished, and every field renders the popover presentation."), each row's status now "Verified current behavior through the internal entry" before the gap; the readiness line says DEC-3 is implemented with that gap; Public concepts states the option is not yet exposed, consumer docs carry noexperimentalentry, and names the internal entry (INV13).verified_bykeeps the sheet tests (they run through the entry) and adds the props-type test and the entry's test.check:knowledgeis aligned.Other callers. Searched Core, Lab, the apps and the CLI for the public option: none.
MobileTokenizer(Lab) is a standalone prototype that composes its own surface, notTokenizer's option; the onepresentation: 'bottom-sheet'hit in Storybook is Selector's own prop. PowerSearch's #8001 uses the public option and is on hold; it is not touched here and will move to the internal entry when it resumes.Writing rubric results
Evidence
TokenizerPropscarriedexperimental.presentation;Tokenizer.doc.mjsdocumented it (en, zh, dense) and three public stories demonstrated it; the pending[experimental]changeset would have announced it in the next release.experimentalonTokenizeror on aTokenizerPropsextension is a type error (@ts-expect-errorin the props-type test); the doc has noexperimentalentry; the public Storybook has no Tokenizer experimental story; the changeset is an internal[chore];TokenizerWithPresentationfrom the internal entry still renders the sheet and passes the whole sheet suite.Tokenizer.test.tsxpasses); the sheet implementation is byte-identical apart from the signature plumbing (Tokenizer.tsxchanges are the props type, the overloads and theTokenizerWithPresentationexport;TokenizerBottomSheet.tsxis untouched).Scope
spec:AST-017FR21 allows its removal in a patch); runtime behavior of every public field is unchanged.Testing
See the validation list in the pull request thread for this head: the seven CI typecheck steps,
experimental-isolation, the Tokenizer suites (sheet 62, SSR 3, props-type,Tokenizer.test.tsx), the internal entry's test, the own-content adopters test,serverSafeComponents,touch-support-bundle, the ESLint plugin'sno-core-internal-importtest, strict lint on the changed files, Prettier,check:repo,check:knowledge,check:changesets, changeset coverage,verify-exportson the builtdist, the docsite previews and build.