Move share button into a snippet #2123

ludoboludo · 2022-11-14T22:31:51Z

PR Summary:

Why are these changes introduced?

Deals with #2043

What approach did you take?

Moved the share button which is repeated 3 times in the theme (feat. product section, main product template and article template).

Other considerations

I could be calling the block parameter something more generic like context in case the share button is used somewhere else than in a block. Thoughts ? 🤔
I could also pass in each parameter which I think would be about 3 of them (block.shopify_attributes, block.settings.share_label, and block.id).

Decision log

#	Decision	Alternatives	Rationale	Downsides
1

Visual impact on existing themes

It should not have any visual impact on existing themes.

Testing steps/scenarios

Test the share button in feat. product section, main-product and main-article (blog post article)
Test changing variants for the feat. product section and product page and make sure the URL is updated.

Demo links

Editor

Checklist

Added PR summary for release notes
Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
Followed theme code principles
Linted with Theme Check
Tested on mobile
Tested on multiple browsers
Tested for accessibility

kmeleta

Couple quick notes

snippets/share-button-block.liquid

ludoboludo · 2022-11-16T20:52:24Z

sections/main-product.liquid

-                    {% render 'icon-share' %}
-                    {{ block.settings.share_label | escape }}
-                  </summary>
-                  <div id="Product-share-{{ section.id }}" class="share-button__fallback motion-reduce">


I've remove this id from this element as I don't see why it was needed. When I search for Product-share nothing comes up in the files.

I also don't see where this would be used so removing it is fine with me. Otherwise I'd suggest just remove the section specific prefix of "Product-" or "Article-"

sections/main-article.liquid

kmeleta

One small suggestion. Otherwise looks good and inline with what I expected.

sections/main-article.liquid

kmeleta · 2022-11-18T18:35:49Z

sections/main-product.liquid

-                    {% render 'icon-share' %}
-                    {{ block.settings.share_label | escape }}
-                  </summary>
-                  <div id="Product-share-{{ section.id }}" class="share-button__fallback motion-reduce">


I also don't see where this would be used so removing it is fine with me. Otherwise I'd suggest just remove the section specific prefix of "Product-" or "Article-"

snippets/share-button-block.liquid

kmeleta · 2022-11-18T18:40:44Z

snippets/share-button-block.liquid

+        <input
+          type="text"
+          class="field__input"
+          id="url"


I see this is preexisting, but do you also feel this is a terrible id?

And also it can be a duplicate if you have a featured product section on the product page 😓
Ill change it.

snippets/share-button-block.liquid

kmeleta

Looks good. Just want to confirm we're all in alignment on using block (rather than context or block_context) as the naming for passing block settings/data? We'll want to try and follow the same convention in all snippets moving forward @eugenekasimov

commit dc81806 Author: Ken Meleta <[email protected]> Date: Tue Nov 22 13:32:46 2022 -0700 Add media sizing settings to featured product (Shopify#2114) commit 9875aea Author: Ken Meleta <[email protected]> Date: Tue Nov 22 13:17:55 2022 -0700 Adjust constrain media values for quick add modal (Shopify#2113) * Adjust constrain media values for quickadd * Adjust offset logic * Minor formatting fix commit 2eb8bf6 Author: Ludo <[email protected]> Date: Tue Nov 22 11:50:50 2022 -0500 Add fixed sizes for some images to prevent errors (Shopify#2087) * remove the mention of sizes where unnecessary * remove another instance * add fixed size to the images in structured data * adjust size to 1920 * edit logo size in structured data * remove conditional info * change value to match other logo size values commit 2f9b9f3 Author: Ludo <[email protected]> Date: Mon Nov 21 15:44:24 2022 -0500 Remove alt attribute in image_tag filter where unnecessary (Shopify#2117) * Add fallback to alt attribute where image_tag filter is used * remove unnecessary use of alt attribute in image_tag filter commit 251e5d9 Author: Ludo <[email protected]> Date: Mon Nov 21 12:18:44 2022 -0500 Move share button into a snippet (Shopify#2123) * Move share button into a snippet * change the property name to context and add details in snippet * address review comments * edit props names and description * Address reviewers comment. Remove unused prop and edit existing ID * move script at the top commit 539af27 Author: Eugene Kasimov <[email protected]> Date: Mon Nov 21 09:01:58 2022 -0800 Prettier liquid files snippets. (Shopify#2115) commit 503fdf8 Author: Eugene Kasimov <[email protected]> Date: Thu Nov 17 08:51:21 2022 -0800 Remove noscript css import for product recommendations (Shopify#2101) commit b2b01fd Author: Eugene Kasimov <[email protected]> Date: Thu Nov 17 08:50:33 2022 -0800 Change variables names (Shopify#2055) commit cf05d0d Author: Francisca Calabria Rubio <[email protected]> Date: Thu Nov 17 16:50:55 2022 +0100 Select text input content on trapFocus (Shopify#2106) * Select text input content on trapFocus * PR fix commit b86d1f3 Author: Ken Meleta <[email protected]> Date: Thu Nov 17 08:50:43 2022 -0700 Change thumbnail media fit to "fill" by default (Shopify#2044) * Change thumbnail media fit to cover by default * Simplify and cleanup thumbnail fit approach commit 8ab8e61 Author: Ludo <[email protected]> Date: Thu Nov 17 10:50:28 2022 -0500 Tweak the image_url size to be the max value where necessary (Shopify#2110) * Tweak the image_url size to be the max value where necessary * fix sizing in the header. commit c337301 Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Nov 17 10:49:52 2022 -0500 Update 1 translation file (Shopify#2130) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit bc8f433 Author: Ken Meleta <[email protected]> Date: Thu Nov 17 08:49:10 2022 -0700 Add constrain media and media fit settings to product page (Shopify#2052) * Add constrain height setting * Add constrain scaling functionality * Add media fit setting and functionality * Quick add override and typo fixes * Update setting language and code cosmetics * Update 8 translation files * Update 5 translation files * Update 3 translation files * Update 3 translation files * Update 1 translation file * Add comment Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>

* Move share button into a snippet * change the property name to context and add details in snippet * address review comments * edit props names and description * Address reviewers comment. Remove unused prop and edit existing ID * move script at the top

ludoboludo added 2 commits November 14, 2022 17:15

Move share button into a snippet

7878877

change the property name to context and add details in snippet

84e40b7

ludoboludo marked this pull request as ready for review November 15, 2022 19:06

kmeleta self-requested a review November 15, 2022 23:55

kmeleta reviewed Nov 16, 2022

View reviewed changes

snippets/share-button-block.liquid Outdated Show resolved Hide resolved

snippets/share-button-block.liquid Outdated Show resolved Hide resolved

snippets/share-button-block.liquid Outdated Show resolved Hide resolved

eugenekasimov self-requested a review November 16, 2022 17:48

ludoboludo commented Nov 16, 2022

View reviewed changes

address review comments

cb1f8cb

ludoboludo requested a review from kmeleta November 16, 2022 21:03

eugenekasimov reviewed Nov 17, 2022

View reviewed changes

sections/main-article.liquid Show resolved Hide resolved

edit props names and description

bfd9b93

ludoboludo requested a review from eugenekasimov November 17, 2022 20:02

kmeleta reviewed Nov 18, 2022

View reviewed changes

Address reviewers comment. Remove unused prop and edit existing ID

094e479

ludoboludo requested a review from kmeleta November 18, 2022 20:46

eugenekasimov previously approved these changes Nov 18, 2022

View reviewed changes

snippets/share-button-block.liquid Outdated Show resolved Hide resolved

move script at the top

4ee87b6

ludoboludo dismissed eugenekasimov’s stale review via 4ee87b6 November 21, 2022 15:17

ludoboludo requested a review from eugenekasimov November 21, 2022 15:17

kmeleta approved these changes Nov 21, 2022

View reviewed changes

eugenekasimov approved these changes Nov 21, 2022

View reviewed changes

ludoboludo merged commit 251e5d9 into main Nov 21, 2022

ludoboludo deleted the snippet-share-button branch November 21, 2022 17:18

Move share button into a snippet #2123

Move share button into a snippet #2123

Uh oh!

Conversation

ludoboludo commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary:

Why are these changes introduced?

What approach did you take?

Other considerations

Decision log

Visual impact on existing themes

Testing steps/scenarios

Demo links

Checklist

Uh oh!

kmeleta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ludoboludo Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

kmeleta Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kmeleta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kmeleta Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kmeleta Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

ludoboludo Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

ludoboludo Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kmeleta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ludoboludo commented Nov 14, 2022 •

edited

Loading