feat: add action menu extension #576
Conversation
ashutosh16
force-pushed
the
toolbar-ext
branch
from
428e144
to
dcfd161
Compare
ashutosh16
force-pushed
the
toolbar-ext
branch
from
dcfd161
to
b6bad4a
Compare
ashutosh16
force-pushed
the
toolbar-ext
branch
from
b6bad4a
to
ea0a3c5
Compare
ashutosh16
force-pushed
the
toolbar-ext
branch
from
a1fc74d
to
1f7389c
Compare
ashutosh16
changed the title
ashutosh16
force-pushed
the
toolbar-ext
branch
from
1f7389c
to
f9bb2bb
Compare
There was a problem hiding this comment.
What is this extension adding? In the screenshot is it the new button? That wouldn't need a change in argo-ui.
I would also strongly advise against CD UI extensions reaching into argo-ui given the deprecated state of this repo per pinned issue #453 . Giving a deprecated repo a new feature that user-facing extensions hook into sounds like exposing far too much, especially for a not very reliable repo.
I would also advise against that as this repo is not supposed to be specific to CD. "extensions" are not a term in this repo either
| <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) => ( |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
See the concrete guidance I left downstream for how to avoid a change in argo-ui: argoproj/argo-cd#19620 (comment)
add toolbar ext add toolbar ext Signed-off-by: ashutosh16 <[email protected]>
Signed-off-by: ashutosh16 <[email protected]> add actionmenut ext Signed-off-by: ashutosh16 <[email protected]> add actionmenut ext Signed-off-by: ashutosh16 <[email protected]>
Signed-off-by: ashutosh16 <[email protected]>
ashutosh16
force-pushed
the
toolbar-ext
branch
from
c75efee
to
09f8799
Compare
|
I think this can be closed given the latest changes in argoproj/argo-cd#19620 that obviate the need for this per my suggestions. CD folks should probably double-check my review there since I'm not a regular contributor there (nor have approver permissions there). Docs have also been added there now |
Ref: argoproj/argo-cd#19620
Screenshot:
