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 misalignment of total items in quick order list #2923

Merged
merged 9 commits into from
Aug 3, 2023

Conversation

eugenekasimov
Copy link
Contributor

@eugenekasimov eugenekasimov commented Jul 28, 2023

PR Summary:

This PR fixes:

  • misalignment of total items element in quick order list.
  • the issue when images were cut in the quick order list if the size of typography was increased higher than 100% .

Why are these changes introduced?

Fixes #2921.

What approach did you take?

  • Set a width for the .quick-order-list__total-itemselement same as the input component and add same margin as padding we use for the input in the table.
  • Changed a bit the way we calculate the padding of the input element to account for icon-info which is always 15px.
  • Added width: 100% to .variant-item__image

Visual impact on existing themes

Testing steps/scenarios

  • Go to a product page and add Quick Order List section.
  • Make sure that total items is aligned with the inputs in the table.
  • Play with different theme settings such as typography (heading and body font sizes), input border and make sure the total-items element is still aligned.
  • Make sure that images in the Quick Order List are not cut.

Demo links

Checklist

@eugenekasimov eugenekasimov changed the base branch from main to font July 28, 2023 23:23
@eugenekasimov eugenekasimov force-pushed the fix-total-misalignment-quick-order branch 2 times, most recently from 23d1a46 to 832236c Compare August 2, 2023 01:38
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 🎉 Added small nitpicks

@@ -562,9 +563,14 @@ quick-order-list-remove-button:hover .icon-remove {
}

.quick-order-list__total-items {
padding-left: calc(((11rem / var(--font-body-scale) + var(--inputs-border-width) * 2)/ 4) + 4.5rem);
width: calc(((11rem / var(--font-body-scale) + var(--inputs-border-width) * 2)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are setting the width, perhaps it makes sense to center align total items to account for long translations:

https://screenshot.click/02-47-l0f8z-3y6qy.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds reasonable. Added here.

sections/quick-order-list.liquid Outdated Show resolved Hide resolved
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.

Sorry, @eugenekasimov... I'm still seeing misalignment on products that have Volume pricing/quantity rules on some variants and not on others.

See reference here: https://screenshot.click/Still-misalignment.png
This product: A/ Image sizing test – 3D models and video
https://os2-demo.myshopify.com/products/test-3d-models-video

FYI, I'm testing and playing with font-scaling and different font styles too in the global settings.

@eugenekasimov
Copy link
Contributor Author

eugenekasimov commented Aug 2, 2023

Sorry, @eugenekasimov... I'm still seeing misalignment on products that have Volume pricing/quantity rules on some variants and not on others.

See reference here: https://screenshot.click/Still-misalignment.png This product: A/ Image sizing test – 3D models and video https://os2-demo.myshopify.com/products/test-3d-models-video

FYI, I'm testing and playing with font-scaling and different font styles too in the global settings.

Good catch @edmund-teh. I updated, it should be fine now.

@sofiamatulis just FYI due to this👆 issue I completely changed my approach and found a way to significantly simplify it. Most of the previous comments are not relevant anymore :)

I bit more context. Previously, the inputs were not aligned between rows if some of them had info-icon and some didn't. So, I aligned them no matter if they have info-icon or not and added the same calculation for total-items. It should good now.

@sofiamatulis sofiamatulis self-requested a review August 2, 2023 17:47
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.

This is working really well now @eugenekasimov! Thanks so much.

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.

Works well! I think well still need to address this: #2923 (comment)

@sofiamatulis sofiamatulis self-assigned this Aug 2, 2023
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.

LGTM!

Base automatically changed from font to main August 3, 2023 14:36
@eugenekasimov eugenekasimov merged commit 42071b5 into main Aug 3, 2023
5 checks passed
@eugenekasimov eugenekasimov deleted the fix-total-misalignment-quick-order branch August 3, 2023 20:10
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
* Fix font for quick order list

* Update opacity

* Set width and margin for total-items

* Add width 100% to image

* Add a rule for product with volume pricing

* Add qty rules as a condition

* Refactor liquid logic

* Roll back changes. Account for 15px info icon.

* Add center alignment for total items

---------

Co-authored-by: Sofia Matulis <sofia.matulis@shopify.com>
This pull request was closed.
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: total misaligned when font size is larger
4 participants