fix(menu): add mt for menu item
#221
Conversation
✅ Deploy Preview for objective-bell-0ffbfb canceled.
|
|
Offline: need to fit design system specs so mt cannot :( |
react/src/theme/components/Menu.ts
Outdated
| @@ -46,6 +46,7 @@ const baseStyle = definePartsStyle((props) => { | |||
| boxShadow: $shadow.reference, | |||
| }, | |||
| item: { | |||
| mt: '0.125rem', | |||
There was a problem hiding this comment.
can see from chromatic that the padding changed. But now that I think more about it, I think it's fine. We should move this into the size section of the theme, then get designers to sign off.
we should also use px in this case, since the border width when highlighting is not using rem
There was a problem hiding this comment.
oki! can i just double check why we wanna put this into sizes rather than baseStyles? (no opinion, cos idk if got diff sizes from the figma + idk if the border will change b/w sizes)
There was a problem hiding this comment.
future proofing, since sizes control the things that may change between sizes.
if border don't change then leave in base lor. but hor, some styles no border. so maybe put in variant safer
There was a problem hiding this comment.
yea i think i'll put this in variant and define a MENU_ITEM_BORDER_STYLES to share b/w outline + clear (i could define a function also but cos we define clear in terms of a function so idk if it would be confusing)
|
Closing, seems like this change not needed. |
Problem
See #188. This problem is because css sizing doesn't account for
boxShadow(as we aren't usingborder)Closes #188
Solution
mtwhich is equal to the size of the box shadow.