canonical / lxd-ui

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

Fixes inability to delete single profiles + minor UI change - [WD-10834] #769

Closed Kxiru closed 1 month ago

Kxiru commented 2 months 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, run as described here.
  2. Perform the following QA steps:
    • To test this fix, navigate to an existing instance -> "Edit Instance" -> View the ability to delete a single instance.
    • Attempting to save an instance with no profile and no alternative disk device specified should result in an error, and an inability to save the changes.
    • One can also create a new instance and view the same feature. Both the instance creation and instance editing pages have the same component.

Screenshots

image

image

webteam-app commented 2 months ago

Demo

Jenkins

demos.haus

mas-who commented 2 months ago

Thanks for the changes :) Just one small question regarding some scss update.

Just leaving the list of comments addressed based on our in person discussion for future reference:

mas-who commented 2 months ago

@Kxiru , there were some fixes from @edlerd that just got merged #771 and #772 . This is a good opportunity to rebase main.

mas-who commented 2 months ago

LGTM :+1:

@piperdeck please have a design pass on this

piperdeck commented 2 months ago

This is looking great!

A couple things:


When a new profile is added, the dropdown defaults to the last profile in the list. This is especially strange behaviour given that the default profile seems to be correctly stickied to the top of the list, so a new profile will never default to the default profile until it's the last profile available. Can we make new attached profiles default to the first in the list rather than the last in the list?

https://github.com/canonical/lxd-ui/assets/45367207/af3ca1f8-300f-4e4c-a7c9-6f6b43f543b5


When the list of profiles overflows the viewport, I need to repeatedly scroll down in order to keep the "Add Profile" button in view. Could we make it so that it scrolls down automatically to keep the bottom of the list visible when you add a new profile?

https://github.com/canonical/lxd-ui/assets/45367207/0057ff21-7ce9-4983-a29a-89c7f44fa216

Kxiru commented 2 months ago

Thanks, @piperdeck for signing off on the existing work. I can also definitely look into adding the additional amendments that you described.

@mas-who @edlerd , for the amendments, should these become a new PR, a new commit, or simply another amendment to the initial commit?

Please advise and I will get started.

mas-who commented 2 months ago

Thanks, @piperdeck for signing off on the existing work. I can also definitely look into adding the additional amendments that you described.

@mas-who @edlerd , for the amendments, should these become a new PR, a new commit, or simply another amendment to the initial commit?

Please advise and I will get started.

These changes are related to this update, so I'd keep it in the same PR.

Kxiru commented 1 month ago

Hi there, @piperdeck , I have implemented your suggestions. Do you mind reviewing the PR so I can grab that sweet Design review +1? Thanks!

piperdeck commented 1 month ago

LGTM