Repository navigation
docs(useInputContainer): add a search field wrapper example block - #9619
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
community
PR Analysis Report🔎 Preview ·
|
meta-cla
Bot
added
the
CLA Signed
f303ab6 to
c5aac90
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the useInputContainer example: the hook visibly drives it, and removing the hook call breaks the icon and padding behaviour as intended. Three things need fixing before it can merge:
- Add
icons: {MagnifyingGlassIcon: 'search', XMarkIcon: 'close'}touseInputContainerHookUsage.doc.mjs, because every bundled block declares the icons it imports andpackages/cli/test/template-icon-declarations.test.mjsfails without it. - Refresh and commit the
search-icon,search-searchandsearch-icon-setgoldens (.jsonand.txt) inpackages/cli/test/__golden__/no-icon-set/, which the new block changes and which fail four tests inpackages/cli/test/experimental-isolation.test.mjs. - Ring the field only while the input has keyboard focus (for example a
:has(> input:focus-visible)outline built fromfocusVars), becausefocusOutlineStyles.focusWithinalso rings the field while the Clear button shows its own ring.
Running the packages/cli tests locally catches the first two.
[Automated review]
| 'A custom search field wrapper with a leading search icon, padding and a trailing Clear button. useInputContainer puts focus in the input when the icon or the padding is clicked, while a click on Clear empties the field without the wrapper taking it over.', | ||
| isReady: true, | ||
| aspectRatio: 4 / 3, | ||
| componentsUsed: ['Button', 'Icon'], |
There was a problem hiding this comment.
Declare the two Heroicons here, so a project's icon set can replace them and packages/cli/test/template-icon-declarations.test.mjs passes:
icons: {MagnifyingGlassIcon: 'search', XMarkIcon: 'close'},| ref={containerRef} | ||
| onClick={onClick} | ||
| onMouseUp={onMouseUp} | ||
| {...stylex.props(styles.field, focusOutlineStyles.focusWithin)}> |
There was a problem hiding this comment.
focusOutlineStyles.focusWithin matches any focused descendant, so when Clear has keyboard focus the field and Clear both show a ring. Scope the field's ring to the input instead, for example:
field: {
// ...
outlineWidth: {default: 0, ':has(> input:focus-visible)': focusVars['--focus-outline-width']},
outlineStyle: {default: 'none', ':has(> input:focus-visible)': focusVars['--focus-outline-style']},
outlineColor: {default: null, ':has(> input:focus-visible)': focusVars['--focus-outline-color']},
outlineOffset: {default: 0, ':has(> input:focus-visible)': focusVars['--focus-outline-offset']},
},focusVars comes from @astryxdesign/core/theme/tokens.stylex.
… only for the input The field's ring now keys off the input's focus-visible, so a focused Clear button shows only its own ring. The search goldens count the new block.
# Conflicts: # apps/sandbox/scripts/next-export-routes.json # internal/shadcn-registry/routes.lock.json # packages/cli/test/__golden__/no-icon-set/search-icon-set.json # packages/cli/test/__golden__/no-icon-set/search-icon-set.txt
There was a problem hiding this comment.
Thanks for the quick turnaround: the icon declaration, the refreshed goldens and the input-only focus ring are all in and the example behaves as described. One change is needed before it can merge:
- Miss: render Clear with
InputClearButtonfrom@astryxdesign/core/Fieldinstead of the hand-built ghostButtoninuseInputContainerHookUsage.tsx(lines 83-93), because the copy loses the shared clear button's "Clear" tooltip, its theme targets and the press handling that keeps focus in the input; then drop theButtonandXMarkIconimports and theXMarkIcon: 'close'entry inicons.
Separately, the brace-only edit to internal/interaction-harness/src/busy.ts is unrelated to the example and can be dropped. CI has not run on this head yet; a maintainer adds the ci:run label to run it.
[Automated review]
| <Button | ||
| label="Clear" | ||
| variant="ghost" | ||
| size="sm" | ||
| icon={<Icon icon={XMarkIcon} />} | ||
| isIconOnly | ||
| onClick={() => { | ||
| setQuery(''); | ||
| inputRef.current?.focus(); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Core already ships this control: InputClearButton from @astryxdesign/core/Field is the clear button every Core input uses, with the localized "Clear" tooltip, the astryx-input-clear-* theme targets and a press that keeps focus in the input. Suggest:
<InputClearButton
label="Clear search"
onClick={() => {
setQuery('');
inputRef.current?.focus();
}}
/>Then drop the Button and XMarkIcon imports and the XMarkIcon: 'close' entry in icons (and update componentsUsed).
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, so a maintainer adds the ci:run label to run it.
Thanks for switching Clear to InputClearButton: the example now uses the same clear control as every Core input, and the tests show the hook drives the icon and padding clicks while Clear keeps its own.
[Automated review]
cixzhang
added
the
ci:run
There was a problem hiding this comment.
Approved: the useInputContainer search field example meets the hook-example rule, its tests show the hook drives the icon and padding clicks while Clear keeps its own, and CI passed on this head.
Thanks for moving Clear to InputClearButton and keeping the field's ring on the input; in Chromium only Clear rings once it has keyboard focus.
[Automated review]
github-actions
Bot
removed
the
needs:code-review
Reader impact
The
useInputContainerdocs page has no runnable example. After this change it shows a custom search field wrapper with a leading search icon, padding and a trailing Clear button. Clicking the icon or the padding puts focus in the input, and clicking Clear empties the field without the wrapper taking over the click.astryx template useInputContainerHookUsage <path>copies it.Source of truth
useInputContainerand its HookDoc (packages/core/src/hooks/useInputContainer.ts,useInputContainer.doc.mjs), including its@examplewrapper.InputClearButton(packages/core/src/Field/InputClearButton.tsx), the clear button Core's inputs share, andTextInput's clear handler, which returns focus to the input after clearing. The example's Clear does the same.architecture:knowledge-contractsINV28.Correction
Adds
blocks/components/Hooks/useInputContainerHookUsage(exampleFor: 'useInputContainer'). A div wrapper holds a search Icon, a text input and, while the field has text, Core'sInputClearButtonlabelled "Clear search".useInputContainer({containerRef, inputRef})returnsonClickandonMouseUp, and both go on the wrapper. The styles are the minimum that makes the div read as one field: a flex row with padding, a border and a radius, a focus ring fromfocusVarsdrawn only while the input itself is:focus-visible(so a focused Clear shows just its own ring), and an unstyled input inside it.Evidence
/components/useInputContainer, 1280x800), run before Clear moved toInputClearButtonand not re-run since. With focus outside the example, a click on the search icon focuses the input. With focus outside again, a click 4px inside the wrapper's right edge (padding) focuses the input. After typing "running shoes", a click on Clear leaves the input empty and focused, and the Clear button is gone.packages/cli/api/template/use-input-container-hook-usage.test.tsx, 3 cases). Asserts focus after a click on the icon and on the wrapper, and that a click on Clear empties the field and calls the input'sfocusexactly once (Clear's own call). With theuseInputContainercall removed, the first two fail. With a plainonClick={() => input.focus()}wrapper instead of the hook, the Clear case fails on the secondfocuscall.Scope
internal/shadcn-registry/routes.lock.jsonits sandbox route inapps/sandbox/scripts/next-export-routes.json, and thesearch icon,search icon setandsearch searchgoldens inpackages/cli/test/__golden__/no-icon-set/.Testing
vitest run --project node packages/cli/foundation/discovery packages/cli/api/template packages/cli/assets/templates internal/shadcn-registry apps/sandbox/src/route-manifest.test.ts packages/cli/api/build/tree.test.mjs: 1168 passed, 0 failed.apps/docsite:pnpm generate, thenvitest run src/__tests__/data-extraction.test.ts src/__tests__/example-coverage.test.ts: 135 passed, 0 failed. The route lock check is clean afterUPDATE_SHADCN_ROUTE_LOCK=1added the new route.vitest run --project node packages/cli/test/experimental-isolation.test.mjs packages/cli/test/template-icon-declarations.test.mjs packages/cli/api/component/resolution-compat.test.mjs: passes. The block'ssearchicon adds one match to thesearch icon,search icon setandsearch searchgoldens.pnpm lint:strict: clean.pnpm -F @astryxdesign/cli typecheck: clean.node scripts/release/changeset-coverage.mjs pr --base upstream/main --head HEAD --released v0.7.8: passes. Changeset is@astryxdesign/clipatch,[docs].pnpm testsuite.Fixes #9568