feat(PopoverConfirm): new component#381
feat(PopoverConfirm): new component#381HaykKirakosyanSoft wants to merge 22 commits intorelease/3.0.0from
Conversation
HaykKirakosyanSoft
requested review from
AregSoft,
GaroGabrielyan and
hamikhambardzumyan
as code owners
…popover-confirm-component"
| @@ -0,0 +1,9 @@ | |||
| .popoverConfirm__title_icon { | |||
There was a problem hiding this comment.
Please rename popoverConfirm__title_icon to a valid BEM-style semantic name.
| open: controlledOpen, | ||
| defaultOpen = false, | ||
| status = "warning", | ||
| ...popoverProps |
There was a problem hiding this comment.
🚨 I think spreading all Popover props into PopoverConfirm makes this component too open and weakens its contract. PopoverConfirm is a specific UX pattern and should expose only the props it actually supports by design. For example, allowed sizes seem limited (small/medium), margin should be internal constant, hasCloseButton appears fixed (false), and options like fitReference are not part of the current design. Could we remove broad prop spreading and define an explicit, constrained PopoverConfirm API instead?
|
|
||
| const primaryButtonAppearance = status === "error" ? "danger" : "primary"; | ||
|
|
||
| const IconComponent = status === "error" ? ErrorFilled : TriangleAlert; |
There was a problem hiding this comment.
Could we use a status-to-icon map instead of a ternary for better scalability/readability? Example:
const iconByStatus = { error: ErrorFilled, warning: TriangleAlert };
const IconComponent = iconByStatus[status];
| [popoverRef.current.floatingElement] | ||
| ); | ||
|
|
||
| const primaryButtonAppearance = status === "error" ? "danger" : "primary"; |
There was a problem hiding this comment.
Could we align this with the same mapping approach to keep status-driven values consistent? Example:
const appearanceByStatus = { error: "danger", warning: "primary" };
const primaryButtonAppearance = appearanceByStatus[status];
2 participants
Chromatic Build Summary
Build URL: https://www.chromatic.com/build?appId=66daee7859a843dd87bfb3d7&number=1242
Storybook URL: https://66daee7859a843dd87bfb3d7-zxaxxhyodn.chromatic.com/