lxd-ui icon indicating copy to clipboard operation
lxd-ui copied to clipboard

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

Open Kxiru opened this issue 9 months ago • 7 comments

Done

  • "Add Profile" button now on a new line whenever there are no profiles in an instance (UI change)
  • Instances with single profiles can now delete the profile as long as an alternative disk device is specified, or the default disk device, overridden.
  • The above applies to when instances are being created and when they are being edited.
  • Changes have only been made to ProfileSelector.tsx.
  • Updated the "Delete" button and it's classname to be "Remove" for better UX.

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

Kxiru avatar May 14 '24 12:05 Kxiru

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:

  • Use css classes instead of inline styling
  • Instead of using <div> to create block elements, use css display: block
  • Instead of having align-items: start on the wrapping div, rather use align-self: start on the button element since its siblings do not need the alignments
  • Some discussions regarding vanilla and how the styling system works

mas-who avatar May 16 '24 04:05 mas-who

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

mas-who avatar May 16 '24 05:05 mas-who

LGTM :+1:

@piperdeck please have a design pass on this

mas-who avatar May 16 '24 10:05 mas-who

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

piperdeck avatar May 16 '24 13:05 piperdeck

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.

Kxiru avatar May 16 '24 14:05 Kxiru

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.

mas-who avatar May 16 '24 15:05 mas-who

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

Kxiru avatar May 20 '24 15:05 Kxiru

LGTM

piperdeck avatar May 23 '24 18:05 piperdeck