chore: [STMS-4898] Add POC of Button component #15
Conversation
SiriusCrain
requested review from
doniyor2109,
lapapooh,
sanjar-sd and
timurhamitov
|
FYI This is just POC, actual styles will be different. |
|
Visit the preview URL for this PR (updated for commit 47e4ead): https://superdispatch-ui-next--pr15-stms-4898-gtm81599.web.app (expires Thu, 16 Oct 2025 07:27:55 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 31dfc7089465d7a81a6dde71dbcc27b0012b1271 |
| props: { variant: "text" }, | ||
| style: textVariant(), | ||
| }, | ||
| { | ||
| props: { variant: "outlined" }, | ||
| style: secondaryVariant(), | ||
| }, | ||
| { | ||
| props: { variant: "contained" }, | ||
| style: primaryVariant(), |
There was a problem hiding this comment.
[critical]: we should use our custom semantic variants as per design:
primary, secondary, success, critical, 'inverted', text
There was a problem hiding this comment.
For now we are discussing naming for buttons
| function textVariant(): CSSProperties { | ||
| return { | ||
| textTransform: "none", | ||
| fontWeight: 500, | ||
| padding: "6px 16px", | ||
| minWidth: 0, | ||
| minHeight: 0, | ||
| fontSize: "14px", | ||
| }; | ||
| } | ||
|
|
||
| function secondaryVariant(): CSSProperties { | ||
| return { | ||
| textTransform: "none", | ||
| fontWeight: 500, | ||
| padding: "5px 15px", | ||
| minWidth: 0, | ||
| minHeight: 0, | ||
| fontSize: "14px", | ||
| }; | ||
| } | ||
|
|
||
| function primaryVariant(): CSSProperties { | ||
| return { | ||
| "&:hover": { | ||
| backgroundColor: ColorDynamic.Red30, | ||
| }, | ||
| textTransform: "none", | ||
| fontWeight: 600, | ||
| padding: "6px 16px", | ||
| minWidth: 0, | ||
| minHeight: 0, | ||
| fontSize: "14px", | ||
| }; | ||
| } |
There was a problem hiding this comment.
Not sure if it makes sense to create separate functions for returning the simple object, we can inline them + considering you have a lot of repetition/common css in all variants, would make sense to define it once in the root style overrides.
There was a problem hiding this comment.
Good catch, will do
| minWidth: 0, | ||
| minHeight: 0, |
There was a problem hiding this comment.
[question]: why 0 for both? I would not expect a minHeight for button, and for minWidth it should be more than 0, at least 64 or smth.
SiriusCrain
force-pushed
the
STMS-4898
branch
from
710e677 to
47e4ead
Compare
2 participants
No description provided.