refactor: use Kobalte for form fields - #144
Merged
Merged
Conversation
jamesarosen
force-pushed
the
refactor-form-fields
branch
from
March 7, 2026 15:04
8490126 to
7dee339
Compare
jamesarosen
force-pushed
the
refactor-form-fields
branch
from
March 7, 2026 18:29
7dee339 to
4df39db
Compare
## Summary - Adopts Kobalte UI (`@kobalte/core`) for form field components across the entire app, including a workaround for `<Select required>` causing inconsistent state. See kobaltedev/kobalte#538 - Establish named JSX props as slot pattern, including a workaround for JSX props inside a `<Show>`. See solidjs/solid#1977 - Migrated `ListingForm`, `InquiryForm`, and `login` to the new form field components - Mitigate open redirect in login form - E2E selectors updated from `input#email` (Kobalte generates its own IDs) to `page.getByLabel(/email/i)` ## Bug Fixes | Severity | Fix | |----------|-----| | CRITICAL | Missing `name="address"` on ListingForm — field was silently dropped from FormData | | CRITICAL | Kobalte `Select` missing `<HiddenSelect />` — fruit type never reached FormData | | HIGH | `validationState` computed outside reactive graph — error styling never applied | | HIGH | Slot props inside `<Show>` broken (solidjs/solid#1977) — fixed with reactive accessor pattern | | HIGH | `FieldErrors.toArray()` called outside reactive context — errors never updated | | HIGH | InquiryForm auto-submit re-fired on remount — clear `inquiry_complete` URL param via History API | | HIGH | InquiryForm auto-submit didn't validate stored `listingId` matches current listing | | HIGH | `returnTo` open-redirect — `/\evil.com` bypassed `startsWith('//')` check; now uses `new URL()` origin check | | MEDIUM | `sessionStorage.setItem` unguarded `QuotaExceededError` | | MEDIUM | `Show when={errors}` truthy for `[]` — empty error container rendered when no errors | | MEDIUM | `Sentry.captureException` called with string instead of `Error` (no stack trace) | | MEDIUM | `itemComponent` now required in `SelectFieldProps` — omitting it caused silent empty listbox | ## Test Plan - [x] Create a listing: fill all fields, submit — fruit type and address appear in confirmation - [x] Create a listing: submit without fruit type — validation error appears - [x] Create a listing: submit without address — validation error appears - [x] Login: invalid `returnTo` like `//evil.com` is rejected - [x] Contact owner: magic link flow, then auto-submit on return - [x] Unit tests: `pnpm --filter @pickmyfruit/www test:run` - [x] Typecheck: `pnpm --filter @pickmyfruit/www typecheck` - [x] E2E: `pnpm --filter @pickmyfruit/www test:e2e` ## Review Notes - Named JSX props confirmed as the correct SolidJS slot pattern (no children classification; SSR-safe) - `<HiddenSelect />` is required for native FormData participation — Kobalte doesn't auto-render it - Kobalte auto-generates input IDs; all test selectors updated to use label/name/role - InquiryForm auto-submit test coverage deferred to #146 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jamesarosen
force-pushed
the
refactor-form-fields
branch
from
March 7, 2026 18:31
4df39db to
ccf5109
Compare
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@kobalte/core) for form field components acrossthe entire app, including a workaround for
<Select required>causinginconsistent state.
See fix(select): fix prop not working properly on hidden select input kobaltedev/kobalte#538
JSX props inside a
<Show>.See Hydration error for rendered Elements that aren't inserted in the DOM during server rendering solidjs/solid#1977
ListingForm,InquiryForm, andloginto the newform field components
input#email(Kobalte generates its ownIDs) to
page.getByLabel(/email/i)Bug Fixes
name="address"on ListingForm — field was silently dropped from FormDataSelectmissing<HiddenSelect />— fruit type never reached FormDatavalidationStatecomputed outside reactive graph — error styling never applied<Show>broken (solidjs/solid#1977) — fixed with reactive accessor patternFieldErrors.toArray()called outside reactive context — errors never updatedinquiry_completeURL param via History APIlistingIdmatches current listingreturnToopen-redirect —/\evil.combypassedstartsWith('//')check; now usesnew URL()origin checksessionStorage.setItemunguardedQuotaExceededErrorShow when={errors}truthy for[]— empty error container rendered when no errorsSentry.captureExceptioncalled with string instead ofError(no stack trace)itemComponentnow required inSelectFieldProps— omitting it caused silent empty listboxTest Plan
returnTolike//evil.comis rejectedpnpm --filter @pickmyfruit/www test:runpnpm --filter @pickmyfruit/www typecheckpnpm --filter @pickmyfruit/www test:e2eReview Notes
classification; SSR-safe)
<HiddenSelect />is required for native FormData participation — Kobaltedoesn't auto-render it
🤖 Generated with Claude Code