Skip to content
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: add action menu extension #576

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/components/top-bar/top-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface Toolbar {
breadcrumbs?: { title: string | React.ReactNode, path?: string; }[];
tools?: React.ReactNode;
actionMenu?: ActionMenu;
actionMenuExtensions?: React.ReactNode[];
}

export interface TopBarProps extends React.Props<any> {
Expand Down Expand Up @@ -85,22 +86,26 @@ const renderBreadcrumbs = (breadcrumbs: { title: string | React.ReactNode, path?
</div>
);

const renderActionMenu = (actionMenu: ActionMenu) => (
const renderActionMenu = (actionMenu: ActionMenu, actionMenuExtensions: React.ReactNode[]) => (
<div>
{actionMenu.items.map((item, i) => (
<button disabled={!!item.disabled} qe-id={item.qeId} className='argo-button argo-button--base' onClick={() => item.action()} style={{marginRight: 2}} key={i}>
{item.iconClassName && (<i className={item.iconClassName} style={{marginLeft: '-5px', marginRight: '5px'}}/>)}
{item.title}
</button>
))}
{actionMenuExtensions && actionMenuExtensions.map((ext, index) => (
Copy link

@agilgur5 agilgur5 Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding another button does not require an argo-ui change, you can just add another item to the list in CD.

EDIT: I left a comment downstream in CD as well: argoproj/argo-cd#19620 (comment)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still want to pursue these changes, and I think this is an easy solution that does not require any extensions or argo. If we have a better solution in the future, I will be happy to review it.

Copy link

@agilgur5 agilgur5 Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an easy solution that does not require any extensions or argo

It's definitely not "easy" given how invasive it is, and it adds the entire concept of extensions to argo-ui (which should not be exposed from an internal repo), which does in fact add logic to Argo

Copy link

@agilgur5 agilgur5 Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the concrete guidance I left downstream for how to avoid a change in argo-ui: argoproj/argo-cd#19620 (comment)

<React.Fragment key={index}>{ext}</React.Fragment>
))}
</div>
);

const renderToolbar = (toolbar: Toolbar) => (
<div className='top-bar row' key='tool-bar'>
<div className='columns small-9 top-bar__left-side'>
{toolbar.actionMenu && renderActionMenu(toolbar.actionMenu)}
{toolbar.actionMenu && renderActionMenu(toolbar.actionMenu, toolbar.actionMenuExtensions)}
</div>

<div className='columns small-3 top-bar__right-side'>
{toolbar.filter && renderFilter(toolbar.filter)}
{toolbar.tools}
Expand Down