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 for small screens with large fonts don't fit all content #2946

Merged
merged 4 commits into from
Aug 11, 2023

Conversation

eugenekasimov
Copy link
Contributor

@eugenekasimov eugenekasimov commented Aug 8, 2023

PR Summary:

This PR fixes an issue when the total price of an item on mobile goes out of screen. This happens when an item has compare_at_price and large fonts, so in that case there is just not enough room to fit the total price on a small screen.

Why are these changes introduced?

Fixes #2931 .

What approach did you take?

I changed the way we display the old price and the current one. They used to be in one line. Now I place them in two different lines under each other.

Visual impact on existing themes

The layout of the old price and the current one has been changed for mobile.

Before 258262137-6e672390-fbc3-4dd7-92c4-1c4cf50082cb
After 258263003-c27329aa-4dc3-42ef-8ce9-4ffc657dc605

Testing steps/scenarios

  • Add the quick order list section to the product page.
  • Go to a product that has compare_at_price (i.e Louise Slide Sandal)
  • In the Quick Order List settings choose Show images.
  • In Global Theme settings set fonts to max
  • in the editor or using the dev tool go to the mobile view.
  • In the quick order list increase the qty of an item that has compare_at_pricer to make the total price be a four-digit number and make sure it fits the screen.

Demo links

Checklist

@andrewetchen andrewetchen self-requested a review August 8, 2023 18:36
@sofiamatulis sofiamatulis self-requested a review August 8, 2023 18:55
Copy link
Contributor

@andrewetchen andrewetchen 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. I left some minor feedback.

@@ -187,6 +187,12 @@ quick-order-list .quantity__button {
align-items: center;
}

@media screen and (max-width: 749px) {
.variant-item__discounted-prices {
display: block;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use flex-wrap: wrap; instead since display: flex; is already being applied here. Additionally, flex-wrap: wrap would allow the prices to be on the same line when there is available space, whereas display: block forces them to be on separate lines (video).

Copy link
Contributor Author

@eugenekasimov eugenekasimov Aug 9, 2023

Choose a reason for hiding this comment

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

Hey Andrew, thanks for a good idea. This makes sense. Fixed.

Choose a reason for hiding this comment

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

Sick! That's a great call, @andrewetchen... sorry, late to catching up with this one. Thank you! 🚀

Copy link
Contributor

@andrewetchen andrewetchen 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!

Copy link
Contributor

@sofiamatulis sofiamatulis left a comment

Choose a reason for hiding this comment

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

Looks good 👌

Copy link

@edmund-teh edmund-teh left a comment

Choose a reason for hiding this comment

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

Tested on my phone, iPad, and on Desktop. All looks great and works as advertised. Thanks @eugenekasimov!

@eugenekasimov eugenekasimov merged commit c2f16b7 into main Aug 11, 2023
5 checks passed
@eugenekasimov eugenekasimov deleted the small-screen-quick-order-list branch August 11, 2023 15:45
TimmersThomas added a commit to TimmersThomas/shopify-template-houseofchocolate that referenced this pull request Aug 13, 2023
* upstream/main: (205 commits)
  Fix for small screens with large fonts don't fit all content (Shopify#2946)
  Adjust quantity rules margin (Shopify#2948)
  Update Social media settings defaults to remove Shopify links (Shopify#2830)
  added json to barcode to pass gtin as a json string (Shopify#2804)
  Fixed extra margin spacing in collage section when header is empty (Shopify#2770)
  Track state of mouseenter event (Shopify#2934)
  Fix misalignment of total items in quick order list (Shopify#2923)
  Hide vol pricing and qty rules when variant is unavailable (Shopify#2889)
  Fix font family for quick order list (Shopify#2888)
  v11.0.0 version bump and release notes (Shopify#2916)
  Revert "v11.0.0 version bump and release notes (Shopify#2906)" (Shopify#2915)
  Update quantity-popover.css
  v11.0.0 version bump and release notes (Shopify#2906)
  Fix social list styles loading (Shopify#2900)
  Fix an error (Shopify#2903)
  Fix hardcoded info color (Shopify#2893)
  Fix error misalignment for Quick order list (Shopify#2887)
  Replace generic section name with section ID. (Shopify#2884)
  Fix cart drawer for variant list and tablet spacing (Shopify#2880)
  Add missing shadow styles to inputs in Quick Order List (Shopify#2879)
  ...
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
…#2946)

* Change display flex to block for mobile

* Revert back from display block to flex. Add flex wrap.
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.

Quick Order list: small screens with large fonts dont fit all content
4 participants