canonical / lxd-ui

Easy and accessible container and virtual machine management. A browser interface for LXD
GNU General Public License v3.0
269 stars 33 forks source link

Allow adding permisisons and identites when creatign a group. Simplify group editing #887

Closed edlerd closed 3 weeks ago

edlerd commented 3 weeks ago

Done

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @mas-who or @edlerd for access.
    • With a local copy of this branch, build and run as described in the docs.
  2. Perform the following QA steps:
    • create a new permission group
    • edit permission group: 1. add identities, 2. remove identities 3. add permissions 4. remove permissions
webteam-app commented 3 weeks ago

Demo

Jenkins

demos.haus

mas-who commented 3 weeks ago

Thanks for these changes! The interaction is a lot more intuitive now :+1: Below are some QA observations so far.

  1. More of a design question. The cancel and confirm buttons are hidden when adding identities and permissions currently, would it not be better to show them anyway? It seems a little weird that I always have to navigate back to the initial groups panel to save the changes.

  2. Another design question, when adding permissions during initial group creation, do we still want to strike out permissions when removing them? Since no permissions would have existed for the group at that point, is it maybe not a little weird?

  3. when trying to remove an identity from a group that is myself, after seeing the warning popup and dismissing it without confirming, the "Save changes" button is stuck in loading state. Screenshot from 2024-09-11 09-48-04 Screenshot from 2024-09-11 09-48-33 Screenshot from 2024-09-11 09-48-55

  4. The rows in the permission groups table seems to have some styling issues, could be caused by the margin on the edit button? Screenshot from 2024-09-11 09-43-14

  5. When removing an identity from a group that is not myself, seems like the warning popup still shows. Saving the changes results in the correct identity being removed though, so I think something might be wrong in the warning check. Actually, seems like if I do any modification to a group with myself in it, then saving changes will result in the warning popup to show. Screenshot from 2024-09-11 09-55-14 Screenshot from 2024-09-11 09-56-14

  6. I see the undo/redo functionality is now removed in the identities and permissions panels when editing a group. I think you did mention this in a separate discussion, but I can't quite remember exactly why. Would you be able to remind me on the reasoning?

  7. Should we disable the "Save changes" button when editing a group if there are no modifications?

edlerd commented 3 weeks ago

Addressed 2, 3, 4 and 5. I think the others need a bit more discussion.

edlerd commented 3 weeks ago

Addressed all the comments, and the design points 1. and 7.

Only remaining is 6. which we probably best discuss in person. In short, the problem with undo is when switching between forms (a new interaction pattern for those panels), a generic undo stack for all 3 forms would be confusing and having 3 different undo stacks seems confusing as well.

mas-who commented 3 weeks ago

Addressed all the comments, and the design points 1. and 7.

Only remaining is 6. which we probably best discuss in person. In short, the problem with undo is when switching between forms (a new interaction pattern for those panels), a generic undo stack for all 3 forms would be confusing and having 3 different undo stacks seems confusing as well.

Thanks! I was thinking about 6. as I reviewed earlier, like you said I think it's not worth the complexity to include undo/redo as a feature here, let's leave it out :+1:

mas-who commented 3 weeks ago

QA looks mostly good! Just one more thing, seems the "Cancel" and "Create group" buttons are still hidden when adding identities or permissions in the create group panel.

mas-who commented 3 weeks ago

thanks for the changes, QA and code both LGTM :+1:

edlerd commented 3 weeks ago

@piperdeck can you have a look at this one? It tries to improve the experience on creation and editing of permission groups.

mas-who commented 3 weeks ago

The delete button for groups is cut off at the edge on smaller screen sizes.

Screenshot from 2024-09-12 14-49-40

edlerd commented 3 weeks ago

The delete button for groups is cut off at the edge on smaller screen sizes.

Good catch! Added a fix for it just now.

mas-who commented 3 weeks ago

LGTM, again :slightly_smiling_face: