-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(feat) lab order actions - Add order-actions-slot and switches icons for overflow menu #25
Conversation
<button | ||
className={classNames( | ||
"cds--btn", | ||
"cds--btn--ghost", | ||
"cds--overflow-menu__trigger", | ||
{ "cds--overflow-menu--open": showMenu }, | ||
styles.overflowMenuButton | ||
)} | ||
aria-haspopup="true" | ||
aria-expanded={showMenu} | ||
id="custom-actions-overflow-menu-trigger" | ||
aria-controls="custom-actions-overflow-menu" | ||
onClick={toggleShowMenu} | ||
> | ||
{menuTitle} | ||
</button> | ||
<div | ||
className={classNames( | ||
"cds--overflow-menu-options", | ||
"cds--overflow-menu--flip", | ||
styles.menu, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch this to move away from specifying carbon classnames manually? Am sure we can get an equivalent from what comes out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pirupius i agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @pirupius @jabahum ...
I initially did that and wrapped the OverflowMenuItem items with OverflowMenu out of the box from carbon , which would be the out of the box equivalent but it does not display the extensionSlot which it houses and its children hence on clicking the icon it displays nothing .
So i used the custom overflow flow menu approach that was used in the patient banner actions . Had discussions with Samuel about this, we tested the 2 options and resolved to keep the custom overflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gitcliff i left a comment that you haven't addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gitcliff r u still pushing updates to this ??
What's pending on this? @gitcliff is there anything else you are working on to complete this work? Thanks |
@gitcliff there comments that were previously added but have not been addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've discussed this and we'll make some UI fixes in follow up PRs
Requirements
Summary
This PR is addressing the issue of adding adding an extension slot under lab order actions to make this more configurable so that implementations can add in whatever custom order actions that suit their needs
Screenshots
Related Issue
Other