✨ add ActionsColumn to Migration waves table #2109
Conversation
Signed-off-by: sarinailinger <sn0533129699@gmail.com>
sarinailinger
requested review from
ibolton336,
sjd78 and
rszwajko
as code owners
sarinailinger
changed the title
There was a problem hiding this comment.
There are few changes in this PR. You could actually split it into 3 small PRs.
We can continue in one PR but please describe the changes:
- adjust the title to better reflect the changes (the more changes the harder to find good words :) )
- list changes in the PR description (first post) here are some examples: 🐛 Fix fetching in InfiniteScroller #2085 or 📖 WSL/Windows setup docs, rework setup docs #2080
I had also few comments/questions to the code (see below)
| </DropdownItem> | ||
| <ActionsColumn | ||
| items={[ | ||
| // { |
There was a problem hiding this comment.
Please remove commented code.
| // }, | ||
| { | ||
| isAriaDisabled: | ||
| migrationWave.applications.length === 0, |
There was a problem hiding this comment.
You use several ways of checking that the applications list is empty - they all OK but for the sake of consistency I would pick one option. My preference would be the one that is already used in the legacy code but feel free to chose.
| what: t("terms.applications").toLowerCase(), | ||
| }), | ||
| onClick: () => { | ||
| if (migrationWave.applications.length > 0) { |
There was a problem hiding this comment.
Do we need this check ? I would expect that isAriaDisabled === true should block clicks.
| // onClick: () => setWaveModalState(migrationWave), | ||
| // }, | ||
| { | ||
| isAriaDisabled: |
There was a problem hiding this comment.
nice! I haven't used it yet but isAriaDisabled does bring some advantages over isDisabled
| { | ||
| isAriaDisabled: | ||
| migrationWave.applications?.length < 1 || | ||
| !hasExportableApplications( |
There was a problem hiding this comment.
do we need to check for both conditions? I haven't tested this flow but looking at the code: when there is no apps then there should also be no exportable apps so the second condition alone would do the job.
the same actions that there is currently should stay
but delete should be red & edit should be in another button(not in the list)
implement it by ActionsColumn