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

fix: publish image from snapshot created in a project that is not the default project #825

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

mas-who
Copy link
Collaborator

@mas-who mas-who commented Jul 31, 2024

Done

  • Fixed the issue where publishing an image from a instance snapshot created in a non-default project fails

Fixes #824

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • Create a new project, use the "Customised" option
    • In the new project, create an instance and then a snapshot for that instance
    • publish an image for that snapshot should work as expected

@webteam-app
Copy link

src/api/images.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this was a bug might be good to add a test case for this, wdyt?

@mas-who
Copy link
Collaborator Author

mas-who commented Jul 31, 2024

As this was a bug might be good to add a test case for this, wdyt?

Yes indeed, will add the test shortly.

@mas-who mas-who force-pushed the fix-create-image-from-snapshot branch from e11c557 to 0e6bd94 Compare July 31, 2024 13:01
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the test! Some ideas to improve on the test code below.

tests/snapshots.spec.ts Outdated Show resolved Hide resolved
tests/helpers/projects.ts Outdated Show resolved Hide resolved
tests/helpers/images.ts Outdated Show resolved Hide resolved
@mas-who
Copy link
Collaborator Author

mas-who commented Jul 31, 2024

whilst trying to fix the failing CI test on firefox, I picked up an issue. The "select all" checkbox is shifted higher usual (see screenshot below)

Screenshot from 2024-07-31 16-06-23

This is caused by this style which I believe was a fix put in previously for safari. Removing it fixes the issue.

Screenshot from 2024-07-31 16-09-27

This is the cause for tests to fail on firefox, since we need to click the input element associated with the checkbox label span. With the span shifted higher, it overlays the input element which causes the click event to fail. To make the test pass, we can add the styling below:

.multiselect-checkbox input.p-checkbox__input {
  bottom: 0.5
}

This would shift the hidden input element up so that the label span no longer overlays the input. But I don't see this being a solution, @edlerd not sure if you have any ideas?

Edit: so adjusting the safari fix by negating the style on firefox works.

// fix for safari https://warthogs.atlassian.net/browse/WD-7486
.multiselect-checkbox span.p-checkbox__label {
  padding-left: 0;

  // applying the above style breaks the checkbox in firefox
  @-moz-document url-prefix() {
    padding-left: 2rem;
  }
}

@mas-who mas-who force-pushed the fix-create-image-from-snapshot branch from 0e6bd94 to 9dca514 Compare July 31, 2024 14:46
@mas-who
Copy link
Collaborator Author

mas-who commented Jul 31, 2024

@edlerd ready for another pass thanks 🙂

Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix and adding a test for it.

@mas-who mas-who merged commit a54c5da into canonical:main Jul 31, 2024
12 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 31, 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.

I cannot create an image from a snapshot
3 participants