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

Adding new icon options to the .json file #1128

Merged
merged 12 commits into from
Jan 25, 2022
Merged

Conversation

danielvan
Copy link
Contributor

@danielvan danielvan commented Jan 7, 2022

Adding new icon names and alternatives we are providing in a future release.
Approach was to keep the icon options in alphabetical order.

Why are these changes introduced?

We want to add flexibility to merchants.

Demo store Editor

What approach did you take?

Kept alphabetical order in the .json files, which means some icons have new placements in the array.

danielvann777 and others added 6 commits January 7, 2022 20:11
Adding new icon names and alternatives we are providing in a future release.
Approach was to keep the icon options in alphabetical order.
@tyleralsbury
Copy link
Contributor

I added the new icons. Two observations.

  1. The new icons are approximately 21x21. This is a good size, considering the detail in the icons themselves. As a result I bumped up the size they render on the page to 2rem instead of 1.6rem. Note that I think they were intended to be 20x20, because some of them are that size, but there are also ones that are 21x20, 20x21, and 21x21. As a result, I decided to use a 21x21 viewbox for the SVG itself.
  2. The older icons were less standardized in size, so some of them look a bit small. Some look okay, but some of them look like they had a 16px viewbox so they render in the top left of the new viewbox (21x21) which makes them look a bit misaligned. We may want to revisit them. Some examples:

Old Icon:
Screen Shot 2022-01-13 at 3 04 59 PM

New Icon:
Screen Shot 2022-01-13 at 3 04 55 PM

@LucasLacerdaUX LucasLacerdaUX self-requested a review January 25, 2022 16:17
@tyleralsbury tyleralsbury self-requested a review January 25, 2022 16:42
Copy link
Contributor

@tyleralsbury tyleralsbury 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 for providing each of them directly in a collapsible tab section! Very helpful 👍

Looks good to me, visually. Code looks good, at least as much as I can actually read of SVGs.

I used inspect element to check the viewbox and the positions of the icons within each viewbox looks good, and I also tested to ensure the slightly larger size didn't throw the collapsible content off, visually. Everything looks good.

Copy link
Contributor

@LucasLacerdaUX LucasLacerdaUX left a comment

Choose a reason for hiding this comment

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

They all look good to me! Tested every icon on both sections with different color palettes and they're all ✅ :DD

@martinamarien martinamarien merged commit 182ebab into main Jan 25, 2022
@martinamarien martinamarien deleted the danielvan-patch-1 branch January 25, 2022 17:54
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
* Adding new icon options to the .json file
* Add icons to product and collapsed content settings

Co-authored-by: martinamarien <martina.marien@shopify.com>
Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
Co-authored-by: Tyler Alsbury <tyler.alsbury@shopify.com>
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.

5 participants