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

announcement bar x mobile drawer alignment #2829

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

lougoncharenko
Copy link
Contributor

PR Summary:

The hamburger menu drawer icon and cart icon on header are aligned with social icons and language/currency selectors on announcement bar

Why are these changes introduced?

Fixes #2748 .

What approach did you take?

Adding padding to the header and adding padding left to the locales

Visual impact on existing themes

Announcement bar and menu drawer will be aligned

Testing steps/scenarios

  • Mobile, tablet, desktop
  • Different browsers
  • Menu drawer l1 l2 and l3 links

Demo links

Checklist

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

This looks good on desktop, but on mobile it looks like the header is narrower than it was before. You can see it cut in here when you resize to a mobile breakpoint:

12-17-mpq30-v4vg7.mp4

I'm not sure we need this PR anymore though — on main, everything looks like it lines up ok:

12-21-cmgzg-4m1vj.mp4

@lougoncharenko
Copy link
Contributor Author

@kjellr, the difference between this PR and main is the shopping cart icon is aligned with the currency and language pickers. But if that isn't important. we can scrap this PR.
Screenshot 2023-07-12 at 10 54 00 AM

@kjellr
Copy link
Contributor

kjellr commented Jul 12, 2023

Ah, ok. Yes I think that's worthwhile. But I don't think we need the changes to sections/header.liquid — they're throwing things off.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

🙌 Thank you!

@lougoncharenko lougoncharenko merged commit f51749d into main Jul 12, 2023
9 checks passed
@lougoncharenko lougoncharenko deleted the announcement-bar-x-mobile-drawer-alignment branch July 12, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Announcement Bar x Mobile Drawer Alignment Issues
3 participants