Skip to content

refactor: use Kobalte for form fields - #144

Merged
jamesarosen merged 1 commit into
mainfrom
refactor-form-fields
Mar 7, 2026
Merged

refactor: use Kobalte for form fields#144
jamesarosen merged 1 commit into
mainfrom
refactor-form-fields

Conversation

@jamesarosen

@jamesarosen jamesarosen commented Mar 7, 2026

Copy link
Copy Markdown
Owner

Summary

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

  • Create a listing: fill all fields, submit — fruit type and address appear in confirmation
  • Create a listing: submit without fruit type — validation error appears
  • Create a listing: submit without address — validation error appears
  • Login: invalid returnTo like //evil.com is rejected
  • Contact owner: magic link flow, then auto-submit on return
  • Unit tests: pnpm --filter @pickmyfruit/www test:run
  • Typecheck: pnpm --filter @pickmyfruit/www typecheck
  • 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 selectors updated to use label/name/role
  • InquiryForm auto-submit test coverage deferred to test(InquiryForm): add coverage for auto-submit after magic-link auth #146

🤖 Generated with Claude Code

@jamesarosen
jamesarosen force-pushed the refactor-form-fields branch from 8490126 to 7dee339 Compare March 7, 2026 15:04
@jamesarosen jamesarosen changed the title refactor: migrate form fields from WebAwesome to Kobalte refactor: use Kobalte for form fields Mar 7, 2026
@jamesarosen
jamesarosen force-pushed the refactor-form-fields branch from 7dee339 to 4df39db Compare March 7, 2026 18:29
## 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
jamesarosen force-pushed the refactor-form-fields branch from 4df39db to ccf5109 Compare March 7, 2026 18:31
@jamesarosen
jamesarosen merged commit b3ea295 into main Mar 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant