ref(ui): make "md" the new buttonBar gap default #95551
Conversation
so that we also have it available without chonk
and adapt the interface to accept the new theme values instead of space util numbers
as it's going to be the new default
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
natemoo-re
left a comment
natemoo-re
left a comment
There was a problem hiding this comment.
Usage changes all LGTM, just curious about the exact type implementation!
| // eslint-disable-next-line boundaries/element-types | ||
| import {space} from 'sentry/styles/space'; | ||
|
|
||
| type Gap = keyof Theme['space']; |
There was a problem hiding this comment.
Should we have our own Space type that doesn't rely on module augmentation from @emotion/react? This will be a large part of our component API surface moving forward.
There was a problem hiding this comment.
I think this just depends where we import theme from. not sure why exactly we augment the emotion types instead of having our own useTheme hook and Theme type 🤷
But yes, we can definitely streamline this type once we have more components that have a prop that needs space
Suspect IssuesThis pull request was deployed and Sentry observed the following issues: Did you find this useful? React with a 👍 or 👎 |
The `ButtonBar` component now: - takes spacing tokens as value for the `gap` prop - defaults to `gap=“md”`, as this were most of the usages That means places where we had `<ButtonBar gap=“1”>` are now just `<ButtonBar>`, whereas we need to pass `gap=“none”` explicitly in places where we want no spacing.
The
ButtonBarcomponent now:gappropgap=“md”, as this were most of the usagesThat means places where we had
<ButtonBar gap=“1”>are now just<ButtonBar>, whereas we need to passgap=“none”explicitly in places where we want no spacing.