Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Apply sx prop to Select's parent for custom styles #3888

Merged
merged 4 commits into from
Nov 2, 2023
Merged

Apply sx prop to Select's parent for custom styles #3888

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64e71e9
apply Select `sx` prop to parent for custom styles
strackoverflow Oct 31, 2023
62fd3ef
deep merge sx prop
strackoverflow Nov 1, 2023
7a7e5fa
add changeset
strackoverflow Nov 1, 2023
ecefb0a
lint fix
strackoverflow Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ const ArrowIndicator = styled(ArrowIndicatorSVG)`
`

const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
({block, children, contrast, disabled, placeholder, size, required, validationStatus, ...rest}: SelectProps, ref) => (
(
{block, children, contrast, disabled, placeholder, size, required, validationStatus, sx, ...rest}: SelectProps,
ref,
) => (
<TextInputWrapper
sx={{
overflow: 'hidden',
Expand All @@ -81,6 +84,7 @@ const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
fill: disabled ? 'GrayText' : 'FieldText',
},
},
...sx,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! This is 99% there.

...sx does a shallow merge with the existing styles so it might accidentally override nested style objects. We have a merge utility for sx that deep merges. Here is a another component that uses merge for reference.

For example, with shallow merge:

<Select sx={{
 '@media screen and (forced-colors: active)': {
    color: 'fg.muted'
 }
}}>

...props.sx will completely overwrite the '@media screen and (forced-colors: active)' key and remove svg styles

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Makes sense!

}}
block={block}
contrast={contrast}
Expand Down
16 changes: 16 additions & 0 deletions src/Select/Select.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,19 @@
</FormControl>
</Box>
)

export const WithCustomStyling = () => (
<Box as="form">
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<Select sx={{borderRadius: '12px', border: '1px dashed #000000'}}>
<Select.Option value="one">Choice one</Select.Option>
<Select.Option value="two">Choice two</Select.Option>
<Select.Option value="three">Choice three</Select.Option>
<Select.Option value="four">Choice four</Select.Option>
<Select.Option value="five">Choice five</Select.Option>
<Select.Option value="six">Choice six</Select.Option>
</Select>
</FormControl>
</Box>
)

Check failure on line 173 in src/Select/Select.features.stories.tsx

View workflow job for this annotation

GitHub Actions / lint 

Insert `⏎`
Loading