[DS-17] Implement Select component + native-only convention pivot #8

Open

chris wants to merge 28 commits from feature/DS-17 into main

pull from: feature/DS-17

merge into: chris:main

chris:main

Conversation 0 Commits 28 Files changed 45 +2072 -929

chris commented 2026-09-03 12:46:53 -06:00

Owner

Copy link

Ticket

DS-17 Implement Select component (Form Inputs category)

Summary

This PR is unusually large in scope for one ticket: implementing Select surfaced a repo-wide
convention decision made during planning, so this branch does both.

1. Repo-wide convention pivot to native-only components

The design-system dropped its dual web (index.tsx)/native (index.native.tsx) implementation
split entirely. Storybook already renders native primitives through a react-native ->
react-native-web alias (.storybook/main.ts), so a single native implementation now covers both
real Expo/RN apps and browser preview — there's no DOM-specific version to hand-maintain per
component. New convention (see CLAUDE.md):

src/<component-name>/
  types.ts
  native/
    index.tsx      # the (only) implementation
    index.test.tsx
    stories.tsx     # titled with the bare component name, flat root namespace

• Text (the only pre-existing component) migrated to this convention in this same PR, so the
  whole repo lands on one consistent pattern rather than a mixed state.
• Build collapsed to a single tsup entry; package.json dropped the react-native export
  condition/field, react-native became a required peer dependency (was optional), react-dom
  dropped out of peerDependencies (stays a devDependency for Storybook).
• .storybook/main.ts gained a second stories glob entry for the literal native/stories.tsx
  filename (doesn't match the default *.stories.tsx pattern).
• CLAUDE.md / README.md updated to describe and justify the new convention.
• Removed the now-dead happy-dom test setup (bunfig.toml preload, src/test-utils/happydom.ts,
  @happy-dom/global-registrator devDependency) — only used by the deleted DOM-based Text tests.

2. Select field component (DS-17 itself)

Built fresh with RN core components only (Pressable, Modal, FlatList, View) — no new UI
dependency, matching Text's zero-dependency precedent. Single-select via a trigger + option
sheet, wired into React Hook Form the same way Text is (useController, inline error rendering).
Styled via NativeWind classNames sourced from existing designSystemPreset tokens.

The only prior "Select" in this stack (swyfft's DOM-only, react-aria-components-based one)
was evaluated and rejected as a port candidate — it bakes in Swyfft-specific business logic and
depends on packages not used here, failing the epic checklist's "no app-specific business logic"
and Expo-compatibility items. It was used only as a UX reference (trigger + option list pattern).

Also added src/nativewind.d.ts — a small local ambient type augmentation adding className?: string to ViewProps/TextProps/PressableProps, since this repo has no nativewind dependency
(by design) and react-native's own shipped types have no className prop. Select is the first
component to actually use className, surfacing the gap; this mirrors NativeWind's own real type
augmentation technique rather than adding the dependency.

Pre-PR review remediation (both an initial full pass and a scoped follow-up re-review over the
remediation diff):

• Fixed: Select's trigger now fires field.onBlur() explicitly on modal close, since RN doesn't
  move focus on a touch press (the trigger's own onBlur prop never fires on a real device).
• Fixed: taps inside the option sheet (not on a row) no longer bubble to the backdrop and dismiss
  without selecting.
• Fixed: field.ref wired onto the trigger, matching Text's convention.
• Fixed: the option sheet now has a bounded max-height so a long options list scrolls instead of
  overflowing.
• Fixed: the Storybook decorator renders <Story /> instead of calling Story().
• Fixed/corrected: a couple of inaccurate code/doc comments (the onBlur reasoning, the
  nativewind.d.ts "scoped" claim, README's claim about dist/ resolving under any bundler).
• Test coverage added/repaired alongside each fix, including a real regression test for the
  sheet-tap-doesn't-dismiss fix (verified by reverting the fix locally and confirming the test
  fails, then restoring it).

Deliberately deferred / out of scope (flagged during review, not fixed here to avoid scope
creep beyond this ticket):

• Adding nativewind as a real devDependency so Storybook renders actual visual styling
  (className is currently inert under Storybook's plain react-native-web render, by design —
  overlaps DS-3, theming architecture, still open).
• Extracting a shared base type for SelectProps/TextProps's duplicated name/label/
  placeholder/rules fields — two components don't yet justify the abstraction.
• A src/text/native/stories.tsx — the approved plan scoped Text's migration to moving existing
  files only.
• Minor accessibility polish (explicit accessibilityState/role on option rows, RNTL mock realism
  for *ByRole queries) — real but not blocking for a first Select implementation.

## Ticket [DS-17](http://192.168.2.100:7123/home/browse/DS-17/) Implement Select component (Form Inputs category) ## Summary This PR is unusually large in scope for one ticket: implementing `Select` surfaced a repo-wide convention decision made during planning, so this branch does both. **1. Repo-wide convention pivot to native-only components** The design-system dropped its dual web (`index.tsx`)/native (`index.native.tsx`) implementation split entirely. Storybook already renders native primitives through a `react-native` -> `react-native-web` alias (`.storybook/main.ts`), so a single native implementation now covers both real Expo/RN apps and browser preview — there's no DOM-specific version to hand-maintain per component. New convention (see `CLAUDE.md`): ``` src/<component-name>/ types.ts native/ index.tsx # the (only) implementation index.test.tsx stories.tsx # titled with the bare component name, flat root namespace ``` - `Text` (the only pre-existing component) migrated to this convention in this same PR, so the whole repo lands on one consistent pattern rather than a mixed state. - Build collapsed to a single `tsup` entry; `package.json` dropped the `react-native` export condition/field, `react-native` became a required peer dependency (was optional), `react-dom` dropped out of `peerDependencies` (stays a devDependency for Storybook). - `.storybook/main.ts` gained a second `stories` glob entry for the literal `native/stories.tsx` filename (doesn't match the default `*.stories.tsx` pattern). - `CLAUDE.md` / `README.md` updated to describe and justify the new convention. - Removed the now-dead happy-dom test setup (`bunfig.toml` preload, `src/test-utils/happydom.ts`, `@happy-dom/global-registrator` devDependency) — only used by the deleted DOM-based `Text` tests. **2. `Select` field component (DS-17 itself)** Built fresh with RN core components only (`Pressable`, `Modal`, `FlatList`, `View`) — no new UI dependency, matching `Text`'s zero-dependency precedent. Single-select via a trigger + option sheet, wired into React Hook Form the same way `Text` is (`useController`, inline error rendering). Styled via NativeWind classNames sourced from existing `designSystemPreset` tokens. The only prior "Select" in this stack (`swyfft`'s DOM-only, `react-aria-components`-based one) was evaluated and rejected as a port candidate — it bakes in Swyfft-specific business logic and depends on packages not used here, failing the epic checklist's "no app-specific business logic" and Expo-compatibility items. It was used only as a UX reference (trigger + option list pattern). Also added `src/nativewind.d.ts` — a small local ambient type augmentation adding `className?: string` to `ViewProps`/`TextProps`/`PressableProps`, since this repo has no `nativewind` dependency (by design) and react-native's own shipped types have no `className` prop. `Select` is the first component to actually use `className`, surfacing the gap; this mirrors NativeWind's own real type augmentation technique rather than adding the dependency. **Pre-PR review remediation** (both an initial full pass and a scoped follow-up re-review over the remediation diff): - Fixed: `Select`'s trigger now fires `field.onBlur()` explicitly on modal close, since RN doesn't move focus on a touch press (the trigger's own `onBlur` prop never fires on a real device). - Fixed: taps inside the option sheet (not on a row) no longer bubble to the backdrop and dismiss without selecting. - Fixed: `field.ref` wired onto the trigger, matching `Text`'s convention. - Fixed: the option sheet now has a bounded max-height so a long `options` list scrolls instead of overflowing. - Fixed: the Storybook decorator renders `<Story />` instead of calling `Story()`. - Fixed/corrected: a couple of inaccurate code/doc comments (the `onBlur` reasoning, the `nativewind.d.ts` "scoped" claim, README's claim about `dist/` resolving under any bundler). - Test coverage added/repaired alongside each fix, including a real regression test for the sheet-tap-doesn't-dismiss fix (verified by reverting the fix locally and confirming the test fails, then restoring it). **Deliberately deferred / out of scope** (flagged during review, not fixed here to avoid scope creep beyond this ticket): - Adding `nativewind` as a real devDependency so Storybook renders actual visual styling (`className` is currently inert under Storybook's plain `react-native-web` render, by design — overlaps DS-3, theming architecture, still open). - Extracting a shared base type for `SelectProps`/`TextProps`'s duplicated `name`/`label`/ `placeholder`/`rules` fields — two components don't yet justify the abstraction. - A `src/text/native/stories.tsx` — the approved plan scoped `Text`'s migration to moving existing files only. - Minor accessibility polish (explicit `accessibilityState`/role on option rows, RNTL mock realism for `*ByRole` queries) — real but not blocking for a first `Select` implementation.

chris added 19 commits 2026-09-03 12:46:53 -06:00

[DS-17] Migrate Text to the native-only lower-hyphen-case component convention. 802ef03911

[DS-17] Collapse the build to a single native-only entry. 517667c0ee

[DS-17] Add a Storybook glob entry for native/stories.tsx. 6425f817c6

[DS-17] Update CLAUDE.md and README.md for the native-only component convention. 92e4de3e8d

[DS-17] Add Select's shared prop types. fd2f05f981

[DS-17] Implement the Select field component. b5e261ae60

[DS-17] Add unit tests for the Select field component. 4223b396aa

[DS-17] Add a Storybook story for Select. c47999de81

[DS-17] Collapse the barrel to a single native-only entry. 9a6eda64c0

[DS-17] Fire field.onBlur() on modal close instead of the trigger's inert onBlur prop. d960e8e0a1

[DS-17] Wire field.ref onto Select's trigger, matching Text's convention. c459621463

[DS-17] Stop taps inside the option sheet from bubbling to the backdrop dismiss. 090eda894c

[DS-17] Bound the option sheet's height so a long options list scrolls. ca6b801fec

[DS-17] Render <Story /> in the Select decorator instead of calling Story(). b43f85199b

[DS-17] Correct nativewind.d.ts's comment: the augmentation is global, not scoped. 9c3af82f9e

[DS-17] Correct README's claim about dist/ resolving in any bundler. ea72d0ceb7

[DS-17] Remove dead happy-dom test setup (unused since the web Text tests were deleted). 0542c047eb

[DS-17] Correct the onBlur comment: a touch press doesn't move focus, Pressable is focusable. 9439a000e1

[DS-17] Add regression test for the sheet-tap-doesn't-dismiss fix. 736c9da479

chris added 1 commit 2026-09-03 13:33:57 -06:00

[DS-17] Rename native/index.test.tsx to native/test.tsx per PR review. ... f2823d763c

bun test's default discovery only picks up filenames with a .test./.spec.
infix, so the bare test.tsx name needs an explicit glob in the test script
to still be found — added `src/*/native/test.tsx` alongside the default
discovery pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

chris force-pushed feature/DS-17 from f2823d763c to b4f1bfd4eb 2026-09-03 13:55:39 -06:00 Compare

chris added 1 commit 2026-09-03 14:01:52 -06:00

[DS-17] Wire up Tailwind CSS processing so Storybook actually renders className styles ... 42a0a8bee6

Select was the first component to use className, and Storybook had no
PostCSS/Tailwind pipeline to compile it — react-native-web passes
className through to the DOM as-is, but with no generated stylesheet
those classes resolved to nothing, leaving the Select story unstyled.

Adds a Storybook-only tailwind.config.ts (dogfooding designSystemPreset)
plus postcss.config.cjs, compiled through Vite's built-in PostCSS
support and imported once via .storybook/preview.tsx. Doesn't add the
nativewind package itself — Storybook's web preview only needs the
compiled CSS, not NativeWind's native className->style compiler; that
stays a consuming Expo app's own concern per the existing
src/nativewind.d.ts precedent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

chris added 1 commit 2026-09-03 14:14:27 -06:00

[DS-17] Wire real NativeWind into Storybook so className actually resolves to style ... 73766d96bd

The Tailwind CSS pipeline from the previous commit was necessary but not
sufficient: react-native-web's View/Pressable/Text/FlatList filter props
through an internal allowlist before render (forwardedProps), and
className isn't in it — the compiled CSS existed but never attached to
any DOM node, so Select still rendered unstyled.

Real NativeWind closes that gap via a Babel transform (nativewind/babel,
built on react-native-css-interop), not a runtime DOM pass-through. Since
@storybook/react-vite has no @vitejs/plugin-react in its own dependency
tree (JSX goes through Vite's esbuild pipeline, which skips Babel
entirely), .storybook/main.ts now adds it explicitly with that preset,
which swaps the JSX pragma so every RN primitive gets intercepted and,
for the ones react-native-css-interop registers (View, Pressable, Text,
FlatList, ...), has its className resolved against the actual compiled
CSS at render time.

tailwind.config.ts adds nativewind/preset (README.md's documented
consumer-side pairing) since react-native-css-interop's runtime expects
the RN-specific variants/utilities it contributes. src/nativewind.d.ts
now references react-native-css-interop's own (more complete) className
type augmentation instead of a hand-rolled one, since nativewind is a
real devDependency now.

Verified end-to-end with a real headless-Chromium screenshot of the
Select story (both closed and open): the rendered DOM carries the actual
Tailwind classes (border-border, bg-surface, rounded-t-lg, ...) and the
component is visibly styled.

nativewind stays a devDependency only — the published package still
ships bare className strings, and a consuming Expo app brings its own
NativeWind/Metro setup per README.md's "Consuming this package" section.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

chris added 1 commit 2026-09-03 14:19:04 -06:00

[DS-17] Slide the Select option sheet up from the bottom instead of fading in at the top ... 8c183e74c9

Modal was fading in with the sheet pinned to the top of the backdrop —
the sheet already had rounded-t-lg (bottom-sheet styling intent), but
nothing positioned it at the bottom or animated it that way. Swaps
animationType to 'slide' (RN's native bottom-sheet transition) and adds
justify-end to the backdrop so the sheet sits at the bottom rather than
stretching to the top by View's default column/flex-start layout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

chris added 1 commit 2026-09-04 07:38:49 -06:00

[DS-37] Implement Overlay component (#10) ... 6e460f163c

## Ticket

[DS-37](http://192.168.2.100:7123/home/browse/DS-37/) Implement Overlay component

## Base branch note

This branch is based on `feature/DS-17` (in Review, unmerged), not `main` — DS-37 needs the
kebab-case/`native/`-only component convention that only exists on that branch. The PR base is set
to `feature/DS-17` accordingly, so this diff shows only DS-37's own commits. Once DS-17 merges to
`main`, this PR's diff (and its future rebase target) will reflect that.

## Summary

- Adds a new `Overlay` primitive (`src/overlay/`) — backdrop, portal (via RN's own `Modal`, aliased
  to `react-native-web`'s `Modal` for Storybook/browser), DOM focus trap, Escape/back-button dismiss
  via `onRequestClose`, and web scroll-locking — that focus-stealing UI (`Select` today; `Modal`,
  `ContextMenu`, `Confirm`, a future `Drawer` later) composes on top of instead of reimplementing.
- Fades in/out by default (`animationType='fade'`), with an `animationType` override for other
  motion (`Select` uses it to keep its prior slide-up sheet).
- Composable API: `backdropClassName`/`contentClassName` for NativeWind-based placement,
  `backdropStyle`/`contentStyle` as an inline-style escape hatch for dynamically-computed anchoring
  (e.g. a future `ContextMenu`), and `dismissible` to gate backdrop/Escape/back-button close for
  confirm-style flows.
- Scroll-lock uses a module-scoped reference count so nested/concurrent overlays (e.g. a `Select`
  opened from inside a `Modal`) don't unlock scroll early when they close out of order.
- Refactors `Select` (`src/select/native/index.tsx`) to render `Overlay` instead of its own
  hand-rolled `Modal` + backdrop/sheet `Pressable`s — same observable behavior, verified by its
  existing 9-test suite (only a `-sheet` → `-content` testID rename).
- Three Storybook stories (`src/overlay/native/stories.tsx`) demonstrate the API generalizing to a
  centered dialog, a bottom sheet, and a non-dismissible confirm-style overlay.
- `src/overlay/native/test.tsx` — 12 tests covering visibility, backdrop/content dismiss, the
  bubble guard, `dismissible={false}`, the `onRequestClose` (Escape/back-button) path, and
  non-LIFO scroll-lock behavior across multiple open overlays.

Verification: `bun run typecheck`, `bun run lint`, `bun run test` (49 pass, 0 fail), and
`bun run build` all pass.

Reviewed-on: #10

chris added 1 commit 2026-09-04 09:15:58 -06:00

[DS-17] Reindent all JS/TS source to 4 spaces and enforce it via ESLint ... 36c4b3f6b2

Switches the project convention from 2-space to 4-space indentation across
every file eslint.config.cjs lints, and adds @stylistic/eslint-plugin's
indent rule (set to 4) so lint now enforces it going forward.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

chris added 1 commit 2026-09-04 09:23:01 -06:00

[DS-17] Reindent package.json and tsconfig.json to 4 spaces ... 2a36848654

Extends the 4-space convention to the two hand-written JSON config files.
The .forgejo workflow YAML and docker-compose.yml were already 4-space, so
no change was needed there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

All checks were successful

Hide all checks

plane-sync-commits / sync-commits (push) Successful in 1s

Details

ci / check (pull_request) Successful in 14s

Details

This pull request has changes conflicting with the target branch.

• eslint.config.cjs
• src/Text/index.native.test.tsx
• src/Text/index.test.tsx

View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Merge commit title [DS-17] Implement Select component + native-only convention pivot (#8)

Merge commit body Reviewed-on: https://git.chrisharrington.me/chris/design-system/pulls/8

[DS-17] Implement Select component + native-only convention pivot (#8) Reviewed-on: https://git.chrisharrington.me/chris/design-system/pulls/8 Copy merge commit message

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feature/DS-17:feature/DS-17

git switch feature/DS-17

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff feature/DS-17

git switch feature/DS-17

git rebase main

git switch main

git merge --ff-only feature/DS-17

git switch feature/DS-17

git rebase main

git switch main

git merge --no-ff feature/DS-17

git switch main

git merge --squash feature/DS-17

git switch main

git merge --ff-only feature/DS-17

git switch main

git merge feature/DS-17

git push origin main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

chris/design-system!8

No description provided.