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 safari regressions [WD-7486] #601

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Conversation

aaryanporwal
Copy link
Contributor

@aaryanporwal aaryanporwal commented Jan 15, 2024

Done

  • Decreased line height for breadcrumb items, because safari renders fonts differently than other browsers.
  • Removed padding-left from selectable main table's checkbox__label, because the padding left was pushing the arrow on the next row in Safari.
  • align-items: center to breadcrumb for because info icon was not aligned perfectly with the breadcrumb.

This changes don't show regressions in chrome, but fixes issues in safari.

Fixes WD-7486

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:
    • Check all breadcrumbs, selectable main tables look okay in safari.

Screenshots

Before:
The screenshots for "Before" are attached in https://warthogs.atlassian.net/browse/WD-7486

After:
Selectable main table:

Screenshot 2024-01-15 at 5 09 17 PM

Breadcrumbs:
Screenshot 2024-01-15 at 5 08 36 PM

[Before]: Breadcrumb with info icon:
image

[After]: Breadcrumb with info icon:
image

@webteam-app
Copy link

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

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, can you rebase on main and see if the tests succeed after that (there was a fix in main for tests)?

@aaryanporwal
Copy link
Contributor Author

I've rebased on main, but it still seems to be failing, couldn't pin point why.

Signed-off-by: Aaryan Porwal <aaryan.porwal@canonical.com>
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.

All good QA and code wise and tests are passing. Thanks for the fixes 👍

@edlerd edlerd merged commit 5fefbf2 into canonical:main Jan 16, 2024
6 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 16, 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.

3 participants