refactor(form-builder): improve routing form and field input components#1736
refactor(form-builder): improve routing form and field input components#1736github-actions[bot] wants to merge 6 commits into
Conversation
|
This PR is being marked as stale due to inactivity. |
github-actions
Bot
changed the title
|
@greptile |
Greptile SummaryThis PR refactors the routing form field rendering by extracting shared components (
Confidence Score: 3/5Not safe to merge as-is: radio button accent colour and checkbox icon colour regress to secondaryColor on all forms where the two colours differ, and the date/time picker components in the calendar field will receive undefined CSS variables in non-underline mode, potentially breaking their appearance. Three concrete wrong-colour bugs were introduced during consolidation: RadioField.accentColor now receives secondaryColor, checkboxStyle.color also receives secondaryColor instead of accentColor, and both ExtendedDatePicker and TimePicker always pass a truthy resolvedUnderlineColor as secondaryColor to getFieldAppearance, which unconditionally injects CSS-variable-based styling classes even when no secondary theme is active and the required CSS variables are not defined. packages/features/form-builder/components/FormFieldInput.tsx (radio/checkbox colour props), packages/app-store/routing-forms/components/calendar/ExtendedDatePicker.tsx and TimePicker.tsx (secondary-theme class guard) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[FormInputFields] --> B[FormFieldRow]
A --> C[FormFieldWrapper]
C --> D[FormFieldInput]
E[FieldRenderer] --> D
D --> F{field.type}
F --> G[text / email / phone / textarea]
F --> H[date DatePicker]
F --> I[calendar CalendarFieldController]
F --> J[select / multiselect]
F --> K[radio / checkbox / boolean]
F --> L[layout divider / heading / paragraph]
D --> M[getFieldAppearance]
M --> N[CSS vars ROUTING_FORM_THEME_CSS]
N --> O[data-routing-form-theme=secondary]
Reviews (1): Last reviewed commit: "chore: Simplify routing form styling cod..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <RadioField | ||
| key={index} | ||
| id={`${field.id}-radio-${index}`} | ||
| label={option.label} | ||
| value={String(option.value)} | ||
| disabled={disabled} | ||
| accentColor={secondaryColor} | ||
| secondaryColor={secondaryColor} | ||
| variant={uiConfig.radioVariant ?? "default"} | ||
| /> |
There was a problem hiding this comment.
accentColor regression
accentColor is set to secondaryColor here, discarding the accentColor prop entirely. In the original FormInputFields.tsx the radio button received the actual accentColor prop (accentColor={accentColor}). Any form where the two color values differ will now show the wrong accent on radio buttons.
| <RadioField | |
| key={index} | |
| id={`${field.id}-radio-${index}`} | |
| label={option.label} | |
| value={String(option.value)} | |
| disabled={disabled} | |
| accentColor={secondaryColor} | |
| secondaryColor={secondaryColor} | |
| variant={uiConfig.radioVariant ?? "default"} | |
| /> | |
| accentColor={accentColor} | |
| secondaryColor={secondaryColor} |
| const checkboxStyle = { | ||
| ...(secondaryColor ? { color: secondaryColor } : {}), | ||
| ...(secondaryColor ? { borderColor: secondaryColor } : {}), | ||
| }; |
There was a problem hiding this comment.
checkboxStyle.color uses wrong prop
color is set from secondaryColor but the original code used accentColor for the color property and secondaryColor only for borderColor. The accentColor prop (available in scope) is completely unused in this style object, causing checkbox ticks/icons to render with secondaryColor instead of accentColor.
| const checkboxStyle = { | |
| ...(secondaryColor ? { color: secondaryColor } : {}), | |
| ...(secondaryColor ? { borderColor: secondaryColor } : {}), | |
| }; | |
| const checkboxStyle = { | |
| ...(accentColor ? { color: accentColor } : {}), | |
| ...(secondaryColor ? { borderColor: secondaryColor } : {}), | |
| }; |
| ? "var(--cal-border-muted)" | ||
| : underlineColor ?? "var(--cal-secondary)"; | ||
| const underlineStyle = isUnderline ? { borderBottomColor: resolvedUnderlineColor } : undefined; | ||
| const { dateButtonClassName, underlineStyle } = getFieldAppearance(variant, resolvedUnderlineColor); |
There was a problem hiding this comment.
resolvedUnderlineColor is always a non-empty string (underlineColor ?? "var(--cal-secondary)"), so getFieldAppearance always receives a truthy secondaryColor. When variant === "default", hasSecondaryTheme becomes true and dateButtonClassName gets classes like bg-[var(--routing-input-surface)] / border-[var(--routing-input-border)]. Those CSS custom properties are only defined under [data-routing-form-theme="secondary"]; without that ancestor they resolve to their initial values, stripping the button of its normal background/border and breaking its visual appearance. The same problem exists in TimePicker.tsx line 45.
| const { dateButtonClassName, underlineStyle } = getFieldAppearance(variant, resolvedUnderlineColor); | |
| const { dateButtonClassName, underlineStyle } = getFieldAppearance(variant, isUnderline ? resolvedUnderlineColor : undefined); |
| }), | ||
| } | ||
| : undefined; | ||
| const { selectControlClassName, selectStyles } = getFieldAppearance(variant, resolvedUnderlineColor); |
There was a problem hiding this comment.
ExtendedDatePicker: resolvedUnderlineColor is always truthy, so secondary-theme CSS-variable classes are always applied. Guard with isUnderline to avoid injecting undefined CSS-variable references in default mode.
| const { selectControlClassName, selectStyles } = getFieldAppearance(variant, resolvedUnderlineColor); | |
| const { selectControlClassName, selectStyles } = getFieldAppearance(variant, isUnderline ? resolvedUnderlineColor : undefined); |
avnotaklu
force-pushed
the
routing_form_improvements
branch
from
71065dd to
b260136
Compare
github-actions
Bot
changed the title
github-actions
Bot
changed the title
1 participant
Summary
This PR refactors and improves the routing form and form builder components, including UI/UX enhancements, styling improvements, and code simplifications.
Changes
Testing Notes