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

added media queries to align the drawer menu with announcement bar #2808

Closed
wants to merge 11 commits into from

Conversation

lougoncharenko
Copy link
Contributor

PR Summary:

Menu drawer hamburger menu icon in the header aligns with social media icons in announcement and language/currency pickers align with cart icon in header in desktop view.

Why are these changes introduced?

Fixes #2748.

What approach did you take?

I added media queries to make the padding left and right in the header match the padding left and right in announcement when in desktop view and shrink it to the previous paddings when in mobile view.

Screenshot 2023-07-10 at 1 29 25 PM

Visual impact on existing themes

Announcement bar social icons and hamburger menu for drawer in the header are aligned

Testing steps/scenarios

  • Test in mobile, desktop, tablet
  • Test to ensure the media queries for the links and paddings are changing as the media size changes (in the inspector)
  • Test for color schemes and gradient to ensure nothing is broken
  • Test in various browser
  • Test with social icons and without
  • Test with language/currency picker and without

Demo links

Checklist

@@ -2304,13 +2299,25 @@ product-info .loading-overlay:not(.hidden) ~ *,
}

@keyframes translateAnnouncementSlideIn {
0% {opacity: 0; transform: translateX(var(--announcement-translate-from))}
100% {opacity: 1; transform: translateX(0)}
0% {

Choose a reason for hiding this comment

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

seems like just spacing changed here and no content- any reason?

Stephanie-Chou and others added 4 commits July 12, 2023 08:45
responds to feedback on readme clarity in #2628

some devs had confusion on which version of dawn to use for client updates.
* Adjust height of announcement bar and sub-items.

* Adjust announcement bar button height on desktop only.

* Fix width of the utility bar on mobile.

* Remove bizarre rem-based values.
* Add shadow opacity to dropdowns and pop-ups

* Change from 10 to 5

Change from 10 to 5
ludoboludo and others added 5 commits July 12, 2023 08:50
* Cart items discount title fix

* add fix to drawer

* make sure it's line level discounts only
* added preset to apply outline button in image-with-text

* Update 10 translation files

* Update 10 translation files

---------

Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
* fix: show accurate subtotal price for cart

* chore: update subtotal in cart drawer

* chore: update copy for estimated total, adjust spacing and placement

* fix: use cart__footer selector for margin instead

* fix: make copy sentence case

* Update 20 translation files

* Update 3 translation files

* Update 7 translation files

---------

Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
* Update auto-rotate for the announcement bar

* Simplify the visibility rule.

* Update 20 translation files

* Auto-advance to the selected message when block is selected.

---------

Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
@@ -272,8 +272,8 @@
"title": "Your cart",
"caption": "Cart items",
"remove_title": "Remove {{ title }}",
"subtotal": "Subtotal",
"new_subtotal": "New subtotal",
"estimated_total": "Estimated total",
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Estimated total for key sections.cart.estimated_total is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@@ -448,7 +448,7 @@
"name": "Announcement bar",
"settings": {
"auto_rotate": {
"label": "Auto-rotate on desktop"
"label": "Auto-rotate announcements"
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Auto-rotate announcements for key sections.announcement-bar.settings.auto_rotate.label is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@lougoncharenko
Copy link
Contributor Author

"I have signed the CLA!"

@lougoncharenko lougoncharenko deleted the align-announcement-bar-x-mobile-drawer branch July 12, 2023 13:25
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.

Announcement Bar x Mobile Drawer Alignment Issues
8 participants