Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make rows in iso and custom-volume modals clickable #673

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Feb 26, 2024

Done

  • Make rows in iso and custom-volume modals clickable

Fixes WD-8689

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:
    • check "choose custom iso" modal on instance creation. Each iso row should have a hover effect, a pointer cursor and be clickable, same experience as the "select image" modal on instance creation
    • check the instance configuration > disk devices > attach custom device modal, row selection should be same as above iso/image selection

@webteam-app
Copy link

Demo starting at https://lxd-ui-673.demos.haus

@mas-who
Copy link
Contributor

mas-who commented Feb 26, 2024

QA looks good! I did however pick up that the custom volume modal have some weird vertical scroll behaviour, I think maybe we should adjust the table height so that the action buttons are always in view? Feel free to skip this one though since it's not in the scope of this PR.

Screenshot from 2024-02-26 20-17-06

Screenshot from 2024-02-26 20-19-59

@edlerd
Copy link
Collaborator Author

edlerd commented Feb 26, 2024

QA looks good! I did however pick up that the custom volume modal have some weird vertical scroll behaviour, I think maybe we should adjust the table height so that the action buttons are always in view? Feel free to skip this one though since it's not in the scope of this PR.

Yes, this was a display bug due to pagination -- which I think shouldn't have been enabled within the modals in the first place. I removed the pagination, and now it should be fixed.

@mas-who
Copy link
Contributor

mas-who commented Feb 27, 2024

QA looks good! I did however pick up that the custom volume modal have some weird vertical scroll behaviour, I think maybe we should adjust the table height so that the action buttons are always in view? Feel free to skip this one though since it's not in the scope of this PR.

Yes, this was a display bug due to pagination -- which I think shouldn't have been enabled within the modals in the first place. I removed the pagination, and now it should be fixed.

Great, thanks for the change, it looks good now! 👍

@piperdeck
Copy link

I like this. The only thing I notice that feels a bit wonky is something that probably doesn't have to do with this PR in particular, but it seems like the breakpoints for switching between the full and compact versions of this table are a little over-tuned. It switches to the compact version when the modal still has lots of room to remain full-sized:

Screen.Recording.2024-02-27.at.2.19.56.PM.mov

Other than that, it looks good to me

…8689

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd edlerd force-pushed the make-selection-modal-rows-clickable branch from 260d004 to 603d84d Compare February 28, 2024 09:02
@edlerd
Copy link
Collaborator Author

edlerd commented Feb 28, 2024

I like this. The only thing I notice that feels a bit wonky is something that probably doesn't have to do with this PR in particular, but it seems like the breakpoints for switching between the full and compact versions of this table are a little over-tuned. It switches to the compact version when the modal still has lots of room to remain full-sized:

This was due to the table configured as responsive, which is not really necessary. I removed that, and it should be fixed now.

@piperdeck
Copy link

Awesome, this is looking good.

@edlerd edlerd merged commit c68421e into canonical:main Mar 5, 2024
10 checks passed
@edlerd edlerd deleted the make-selection-modal-rows-clickable branch March 5, 2024 07:42
github-actions bot pushed a commit that referenced this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants