-
Notifications
You must be signed in to change notification settings - Fork 6
fix: various props in field components #901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 9f2d96d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📦 NPM canary releaseDeployed canary version 0.0.0-canary-e64c14a. |
🏋️ Size limit report
Click here if you want to find out what is changed in this build |
🧪 Storybook is successfully deployed!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Destructured label/field styles lost in spread
The component destructures labelStyles from props earlier (line 153), then passes ...props to wrapWithField without explicitly including labelStyles. Since destructuring removes the property from the spread, labelStyles is lost and won't be applied to the field. The same pattern affects other components like Checkbox, RadioGroup, DateInput, and others that destructure labelStyles or fieldStyles then spread props without reincluding them.
src/components/fields/Switch/Switch.tsx#L215-L226
cube-ui-kit/src/components/fields/Switch/Switch.tsx
Lines 215 to 226 in c1e9edd
| return wrapWithField(switchField, domRef, { | |
| ...props, | |
| id, | |
| labelProps: { | |
| ...props.labelProps, | |
| for: id, | |
| }, | |
| children: null, | |
| inputStyles, | |
| }); | |
| } |
Bug: Destructured label/field styles lost in spread
The component destructures labelStyles from props (line 278), then passes ...props to wrapWithField without explicitly including labelStyles. Since destructuring removes the property from the spread, labelStyles is lost. The old code explicitly passed styles: labelStyles to compensate; removing this without adjusting destructuring causes styling to be lost.
src/components/fields/Select/Select.tsx#L490-L501
cube-ui-kit/src/components/fields/Select/Select.tsx
Lines 490 to 501 in c1e9edd
| return wrapWithField<Omit<CubeSelectProps<T>, 'children'>>( | |
| selectField, | |
| ref, | |
| mergeProps( | |
| { | |
| ...props, | |
| }, | |
| { labelProps }, | |
| ), | |
| ); | |
| } |
Bug: Destructured label/field styles lost in spread
The old code explicitly passed labelStyles and styles to wrapWithField despite them being destructured from props. The new code removes these explicit passes, relying on ...props to include them. However, destructuring removes these properties from the remaining spread, causing them to be lost. Components that extract labelStyles and fieldStyles must either avoid destructuring them or explicitly repass them.
src/components/fields/Checkbox/Checkbox.tsx#L251-L257
cube-ui-kit/src/components/fields/Checkbox/Checkbox.tsx
Lines 251 to 257 in c1e9edd
| if (!groupState) { | |
| return wrapWithField(checkboxField, domRef, { | |
| ...props, | |
| children: null, | |
| inputStyles, | |
| }); | |
| } |
Bug: Destructured label/field styles lost in spread
The component destructures styles from props and applies it to RadioGroupElement. When passing to wrapWithField, the old code explicitly passed styles: props.fieldStyles. The new code removes this, but props.fieldStyles would still need to be explicitly passed to survive destructuring of the original styles variable. Using ...props alone loses destructured properties.
src/components/fields/RadioGroup/RadioGroup.tsx#L160-L167
cube-ui-kit/src/components/fields/RadioGroup/RadioGroup.tsx
Lines 160 to 167 in c1e9edd
| return wrapWithField(radioGroup, domRef, { | |
| ...props, | |
| children: null, | |
| fieldProps, | |
| labelProps: mergeProps(baseLabelProps, labelProps), | |
| }); | |
| } |
| isDisabled, | ||
| necessityIndicator, | ||
| labelProps, | ||
| labelProps: mergedLabelProps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Field wrapper element loses styling from extracted props
The wrapWithField function no longer receives or passes the styles prop to FieldWrapper, causing the field wrapper element to lose styling. Field components previously passed extracted styles from extractStyles() to wrapWithField, which passed them to FieldWrapper.styles. Removing styles from the WrapWithFieldProps interface and not extracting it breaks the styling pipeline for the field wrapper container element.
Note
Unifies field/label props handling across field components and adds onOpenChange to Picker/FilterPicker/ComboBox/Select, removing deprecated wrapperStyles.
fieldProps,fieldStyles,labelProps,labelStylesinwrapWithField(shorthand styles take priority);FieldWrapperapplies mergedfieldProps.Checkbox,CheckboxGroup,RadioGroup,TextInputBase,FileInput,Date*inputs/pickers,SliderBase,Switch,ListBox,FilterListBox,FilterPicker,Picker,ComboBox,Select) to rely on unified field/label prop handling and remove redundantstylesforwarding to the field wrapper.wrapperStylesinTextInputBaseandSelect(usestyleson root instead).onOpenChange?: (isOpen: boolean) => voidtoPicker,FilterPicker,ComboBox,Select; invoke on popover state changes.src/components/fields/README.mdoverview.Slider.stories.tsxto usefieldStylesfor width.Written by Cursor Bugbot for commit 9f2d96d. This will update automatically on new commits. Configure here.