Delete modal - create a shared delete modal, use for all deletions #312
Conversation
using title + titleVariant instead of a custom modal header fixes the font size and icon alignment, lowercasing the second word (Delete user => Delete group) and adding a description with the name in bold
otherwise, `git commit` fails when a file is being edited, because prettier fails with
[error] No parser could be inferred for file: src/containers/group-management/.group-detail.tsx.swp
also behaves the same way, alerts and redirects to group list after success
|
@sbuenafe-rh can you review please? :) This should be unifying all the galaxy UI removal/deletion confirmation modals and aligning them with patternfly guidelines. (And if you have any input on the question of deleting a group with users inside... :)) |
|
@himdel The delete user and remove user from group modals look good. Deleting a group's message needs to be reworked a little to mention that the users in this group will lose access. Not sure if we can list out the users... |
|
@sbuenafe-rh thanks :) would something like this make sense? for a bit, if the users list is not already loaded (the blue bit is a small spinner, spinning):
empty group:
nonempty group:
Any ideas how to deal with groups with 10+ users? (WIP: right now this is only implemented on the group detail screen, and not limiting the number of users to display in the list, nor the list height; and the list should be sorted) |
wip: deal with more users, list screen
|
@himdel looks good! For 10+ users, we could just state that deleting this group will affect [X] users. |
default sort pretended to be username but the list was unsorted until the sort order was changed and there was only 1 page because the count came from results length, limited by page size
Introduced in ansible#248, PageHeader wraps logo in `a` by default and the logo became a Link, leading to `a > a > img` We can override the upper `a` using `logoComponent` (https://www.patternfly.org/v4/components/page#pageheader), doing that.
…own modal, use from group-list
|
Thanks, updated... :) For up to 10 items...
Over 10...
I've also fixed the Users tab in group detail to show more than 10 items, sorted by username, Ready for more review :) |
| <Link | ||
| to={formatPath(Paths.searchByRepo, { | ||
| repo: this.state.selectedRepo, | ||
| })} | ||
| > | ||
| <SmallLogo alt={APPLICATION_NAME}></SmallLogo> | ||
| {children} |
|
@himdel When checking for affected users...will the Delete button be disabled? |
|
@sbuenafe-rh Right now, it won't be disabled. My assumption was that you do know that you want to delete, and don't necessarily want to wait for the list of users to load. But it can be easily disabled if you prefer :) |
Fixes https://issues.redhat.com/browse/AAH-358
This unifies the confirmation modal for all deletions/removals in the app so that we're:
titleVariant='warning'to fix the icon alignment in the headerIt also adds a missing Delete button to the group detail screen (user detail has it),
tweaks
.gitignoreand.prettierignoreto ignore vim swap files,fixes a logo console warning,
and updates the Users tab in Group detail to work with 10+ users.
(I can split these off if needed :).)
Before:
Users > kebab Delete - icon alignment, missing bold
Users > detail > Delete - icon alignment, missing bold
Groups > Delete - icon alignment, missing description
Groups > detail > Delete - no such button
Groups > detail > tab Users > kebab Remove - no confirmation modal, removed directly
After:
Users > kebab Delete
Users > detail > Delete
Groups > Delete
(see comments for a version with a list/count of users)
Groups > detail > Delete
(see comments for a version with a list/count of users)
Groups > detail > tab Users > kebab Remove
Question: right now, a group can be deleted even if it has users assigned to it. Should that be possible? Warning at least?Resolved by showing a list of affected users.