Redesign RSVP field type as a substrate, with displayAs as the widget - #215
Open
qiyundai wants to merge 3 commits into
Open
Redesign RSVP field type as a substrate, with displayAs as the widget#215qiyundai wants to merge 3 commits into
qiyundai wants to merge 3 commits into
Conversation
…m parity The RSVP field type picker only covered text/email/phone/select/checkbox — missing number/date/url/text-area, which the events-form renderer already supports as native inputs/textarea. Add them to the type union and picker, generalize the options-clearing logic to key off "is a choice type" instead of enumerating non-choice types, and defensively normalize any pre-existing `multi-select` field data (predates ESP's type enum validation) on every scope-config read path so it renders as a checkbox/multi-select instead of silently losing its options. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oduce it ESP's ScopeConfigRsvpField.type enum has never included multi-select (verified against ESP's git history since the schema was introduced), and every write path is behind request validation, so no scope-config read from ESP could ever contain type: 'multi-select'. normalizeRsvpField was defending against a scenario that can't occur; the existing inferFieldType/inferDisplayAs handling in rsvpFieldDefinitions.ts (for the separate legacy per-cloud-sheet path, which is real external data) is unaffected and still needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sharmeebuilds
approved these changes
Jul 16, 2026
sharmeebuilds
left a comment
sharmeebuilds
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — verified type-check and lint clean on the branch, and traced getDisplayAsOptions/getDefaultDisplayAs to confirm they already key off select/checkbox only, so the four new types correctly fall through to no displayAs options without any further changes needed there. The isChoiceType generalization is a nice cleanup — keeps the options-clearing logic correct by construction as new types get added, instead of needing every future type enumerated at each call site.
One non-blocking flag: per the PR description, saving these new types will 400 until ESP #1111 lands (enum widening), and the manual round-trip checkbox is unchecked for the same reason. Given that's just a sequencing constraint rather than a code issue, approving — please just make sure merge/deploy is coordinated with ESP #1111 as noted.
Approving ✅
Narrows RsvpFieldType from a 9-value union (one per field flavor) to text|select|multi-select — the fundamental control kind a field needs. RsvpDisplayAs now carries every flavor/widget within a substrate (email/phone/number/date/url/text-area for text; picker/radio for select; checkbox/combobox for multi-select), matching ESP's mirrored schema change. The Display-As picker now appears for text fields too (new UX — pick a text flavor), and every select/checkbox cardinality check in ConfigManagement.tsx and rsvpFieldDefinitions.ts (options gating, expandable rows, isSelectableField) becomes select/multi-select. Typecheck passing after this narrowing is itself the completeness check — every stale flavor-as-type literal became a compile error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merged
3 tasks
qiyundai
changed the title
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
2 participants
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
Resolves MWPW-200890
RsvpFieldTypeto exactlytext | select | multi-select— the fundamental substrate a field needs, matching ESP's mirrored schema change (ESP PR).RsvpDisplayAsnow carries every flavor/widget within a substrate:text | email | phone | number | date | url | text-areafortype=text;picker | radiofortype=select;checkbox | comboboxfortype=multi-select.select/checkboxcardinality check inConfigManagement.tsxandrsvpFieldDefinitions.ts(options gating, expandable rows,isSelectableField) becomesselect/multi-select.inferFieldType/inferDisplayAs(legacy per-cloud-sheet mapping) updated to the same taxonomy.RegistrationFieldsComponent.tsxneeds no changes — it only depends onisSelectableField.Coordination note: should merge/deploy after or alongside ESP's PR, which this schema depends on — if this ships first, saving a field will 400 until ESP's narrowing is live. event-libs needs a matching remap extension: events-form.js, rsvp-form block.
Test plan
npm run type-checkpasses — the narrowed union turns every stale flavor-as-type literal into a compile error, which doubles as a completeness checknpm run test:unitpasses (73/73)Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com