fix(select): fix prop not working properly on hidden select input#538
Open
royeden wants to merge 1 commit intokobaltedev:mainfrom
Open
fix(select): fix prop not working properly on hidden select input#538royeden wants to merge 1 commit intokobaltedev:mainfrom
royeden wants to merge 1 commit intokobaltedev:mainfrom
Conversation
👷 Deploy request for kobalte pending review.Visit the deploys page to approve it
|
|
This also happens with the Combobox HiddenSelect. For anyone who wants to fix this in user-land while we wait, this is working for me: function MySelect(props) {
/**
* If `props.required`, <HiddenSelect> renders an `<input required>`, but it
* doesn't update its value.
* @see https://github.com/kobaltedev/kobalte/pull/538
*/
const syncHiddenInputOnChange = (e: HTMLElementEventMap['change']) => {
const select = e.currentTarget as HTMLSelectElement | undefined
if (!select) return
const input = select.parentElement?.querySelector('input')
if (!input) return
input.value = select.value
}
let hiddenSelect: HTMLSelectElement | undefined
onMount(() => {
hiddenSelect?.addEventListener('change', syncHiddenInputOnChange)
onCleanup(() => {
hiddenSelect?.removeEventListener('change', syncHiddenInputOnChange)
})
})
// ...
return (<Root {...props}>
<HiddenSelect ref={hiddenSelect} />
...
</Root>)
} |
jamesarosen
added a commit
to jamesarosen/PickMyFruit
that referenced
this pull request
Mar 7, 2026
## 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 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jamesarosen
added a commit
to jamesarosen/PickMyFruit
that referenced
this pull request
Mar 7, 2026
## 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>
8 tasks
jamesarosen
added a commit
to jamesarosen/PickMyFruit
that referenced
this pull request
Mar 7, 2026
## 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>
|
Actually, that doesn't quite work because it doesn't sync on mount. I tried extracting |
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.
Hello!
I've noticed that
<Select />components don't honor therequiredattribute properly, so this PR assigns avalueattribute (same one assigned to theselectelement) to theinputinside of<HiddenSelect />.I wanted to add a test for this issue but I ran into the
pointer-eventsissue that makes you skip all the currentSelecttests.