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

Allow bulk select/delete images in a project and unify pagination placement #620

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Jan 22, 2024

Done

  • allow bulk select and delete images
  • extract PageHeader component
  • refactor settings, instance list and project list to use the new header component
  • move table pagination always below table

Fixes #618

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 @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • select one or many images and remove them from a project's image list
    • ensure layout of following pages (also responsive and with panel) is correct:
      • instance list
      • instance detail snapshots
      • profile list
      • image list
      • settings
      • storage volume list
      • custom storage volume snapshot list
      • custom iso list

@webteam-app
Copy link

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

@mas-who
Copy link
Collaborator

mas-who commented Jan 23, 2024

I think we should align the page layout between images list and instances list? On the images page, when we bulk select, the delete button appears towards the right hand side of the page where on the instances page, the same action results in the delete button appear where the search bar is located (next to the other actions buttons). Maybe we can align layout and behaviour with the instances page?

Images list
Screenshot from 2024-01-23 09-43-39
Screenshot from 2024-01-23 09-43-50

Instances list
Screenshot from 2024-01-23 09-44-10
Screenshot from 2024-01-23 09-44-19

src/api/images.tsx Outdated Show resolved Hide resolved
@edlerd edlerd force-pushed the add-bulk-delete-images branch 3 times, most recently from 562ca76 to d8570ed Compare January 23, 2024 10:30
@edlerd edlerd force-pushed the add-bulk-delete-images branch 3 times, most recently from 8f070e0 to 77cc8f1 Compare January 23, 2024 13:14
@edlerd edlerd changed the title Allow bulk select/delete images in a project Allow bulk select/delete images in a project and unify pagination placement Jan 23, 2024
@piperdeck
Copy link

This looks good to me, but I'm still not sure we should move the table pagination to the bottom of the page. For one thing, the reason we moved it to the top was because I observed users being confused and not finding the pagination controls when they were at the bottom, and for another thing, this might interact negatively with the new notifications which float at the bottom of the page.

Could we talk about this next week in a working together session?

@edlerd
Copy link
Collaborator Author

edlerd commented Jan 26, 2024

This looks good to me, but I'm still not sure we should move the table pagination to the bottom of the page. For one thing, the reason we moved it to the top was because I observed users being confused and not finding the pagination controls when they were at the bottom, and for another thing, this might interact negatively with the new notifications which float at the bottom of the page.

Could we talk about this next week in a working together session?

My main idea was to make the pagination placement consistent. In our discussion we concluded, due to users having trouble finding the select notifications if the pagination (with select info) was rendered below, to move all of them up.

Now, all TablePaginations are used with their default position of above.

Thank you for the fruitful discussion @piperdeck and @mas-who please give it another look as it is now.

@mas-who
Copy link
Collaborator

mas-who commented Jan 29, 2024

QA looks good, just some code review comments for consideration 🙂

@mas-who
Copy link
Collaborator

mas-who commented Jan 29, 2024

LGTM. Thanks for making the changes :)

@piperdeck
Copy link

When I select an item in the list, the delete button is appearing off to the side for some reason

image

This should be "created at"

image

Since this table's rows will always be at least 2 lines of text tall, maybe this description field should be able to occupy 2 lines before being truncated?

image

Other than that, this is looking good :)

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
Copy link
Collaborator Author

edlerd commented Feb 1, 2024

Addressed the three issues you found @piperdeck -- please give it another pass.

@piperdeck
Copy link

Everything looks good!

@edlerd edlerd merged commit 7331f0a into canonical:main Feb 3, 2024
6 checks passed
@edlerd edlerd deleted the add-bulk-delete-images branch February 3, 2024 14:35
github-actions bot pushed a commit that referenced this pull request Feb 3, 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.

[FR] Allow deleting images by selection box/checkbox, similar to how deleting instances is today
4 participants