feat: add Card variant #2332
feat: add Card variant #2332
Conversation
|
@zihanwang-okta Generally speaking, code looks pretty good to me. As a whole, I think it makes more sense to have an entirely different icon for when this button does something out of the ordinary. Feels a bit strange to me for seemingly the same button to provide different interactions in different situations. The user wouldn't know what to expect. I know @jordankoschei-okta had some thoughts/ideas about how to implement this as well. |
|
I believe we landed on something like adding an If we wanted to take that implementation to the next level: We could create an HOC component like |
|
I think this should use a different icon if it's going to open a drawer. Every time I click the 3-dots in Okta, don't feel right when the sidebar pops open. I expect to see an actions menu. |
@KevinGhadyani-Okta Any thoughts on this approach? This would be how I'd probably attack it but wanted to try and get alignment on approach before @zihanwang-okta writes more code |
| const openMenu = useCallback<MenuContextType["openMenu"]>((event) => { | ||
| if (onClickMenuButton) { | ||
| onClickMenuButton(event as React.MouseEvent<HTMLButtonElement>); | ||
| return; | ||
| } | ||
| setAnchorEl(event.currentTarget); | ||
| }, []); |
There was a problem hiding this comment.
I don't know what MenuContextType["openMenu"] means, but we shouldn't have to ever use as on an event. The type can be set in the useCallback generic: useCallback<MouseEventHandlerK<HTMLButtonElement>>.
| render: ({ ...props }) => ( | ||
| <Box sx={{ maxWidth: 262 }}> | ||
| <Card | ||
| {...props} | ||
| overline={props.overline} | ||
| title={props.title} | ||
| description={props.description} | ||
| onClickMenuButton={() => alert("Clicked on menu")} | ||
| onClick={() => alert("Clicked on Card")} | ||
| /> | ||
| </Box> | ||
| ), | ||
| }; |
There was a problem hiding this comment.
Instead of using ...props here, it'd be better to be explicit about what props we're passing.
Using {...props} makes it hard to see what props are being passed into Card.
|
@jordankoschei-okta Is this PR still needed or did your other work cover it? |
|
@KevinGhadyani-Okta The new component |
OKTA-794818
Summary
...at the top right corner) to open the side drawerTesting & Screenshots