Share button UI update #349

ludoboludo · 2021-08-09T17:42:15Z

Why are these changes introduced?

Fixes #204

What approach did you take?

Edited the styling and some of the JS functionality.

Other considerations
Is there classes I could reuse that I didn't 🤔

Demo links

Store
Editor

Checklist

ludoboludo · 2021-08-09T18:46:57Z

locales/en.default.json

@@ -44,6 +44,7 @@
      "item_added": "Item added to your cart"
    },
    "share": {
+      "close": "Close share popup",


@Oliviammarcello what could be the copy/content here 🤔 it's basically to announce the role of the button to screen reader.

@gretchen-willenberg Do you have any thoughts on what this text should be?

I don’t think we have to include popup since that’s just the container vs the role/action. But, it would be good to align it to the initial state. What is that?

I’m guessing….
Action: I don’t like this literal action of the UI vs what user intent is
V1. Open share
Role:
v1. Share product
v2. Share this product

Closing the popup
Role :
v1. Close
See Polaris
Action:
V1. Close share

Additional feedback: Exclamation marks should be celebratory, and copying to clipboard doesn’t meet that threshold to me. Can we remove the ! And just say Copied to clipboard

Side take: Let's track the Shares for Blog post: My gut reaction is that end-user customers/readers are less likely to share a blog post before they read it? Under the title seems very immediate and gives users an action before reading the blog post…which to me is the primary action of visiting the page. I’m hesitant to drive attention away so quickly. On the other hand, it’s clearly secondary and not overly dominating. Use will tell us!

Good points 🤔 👌 Thanks for the 👀

sofiamatulis

Nice :) Added a few comments from testing and will look at the code afterwards

assets/base.css

sofiamatulis · 2021-08-09T18:56:29Z

assets/base.css

+}
+
+.share-button__message:not(:empty):not(.hidden) ~ * {
+  display: none;
 }



Scroll sideways when input isnt selected.

Click outside the input.

Open the popup again.

Youll see the input empty

https://screenshot.click/09-55-2jdow-wsg2v.mp4

I hadn't replied to this since it's related to your previous comment. I will take a look and see why it's happening :)

Seems like an edge case. I tested using Opera on mobile and it used the web share API. iOS supports it as well, Chrome, Firefox and Samsung internet on mobile.

It's only when using the inspect tool and the mobile simulator. I feel like it's probably ok to skip this. What do you think ?

Sounds good :) We can create an issue as an enhancement, but I agree it's an edge case

Actually this seems to be happening on desktop too: #349 (comment)

Looks like it's a chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=596804
When I try on Firefox it's not doing as well.

sections/main-article.liquid

Oliviammarcello · 2021-08-09T20:04:25Z

Looking good, I have a few notes:

The icon is slightly misaligned. Screenshot
On hover the text and icon should change to 100% opacity and have an underline. When the popup is enabled, the 100% should remain
Reduce padding around share. Screenshot
The copy icon is not aligned to the center
The popup is slightly out of the margins. Screenshot.
The max-width for the popup should be 310px. It shouldn't fill the width of the column. Figma.

I added notes for the featured product share update here. It can be tackled in that issue if its easier, but just an FYI

sofiamatulis

Looks great 👍 Added a few small comments and will do another pass

assets/base.css

sofiamatulis · 2021-08-10T19:39:57Z

templates/article.json

-       "content",
-       "share"
+       "share",
+       "content"
    ]
   }
 },


Is there a console log I am missing? 🤔 I dont see it on main

https://screenshot.click/10-38-3enf1-lhc0l.png

oh this is coming from the blog post itself due to how it's setup in the admin:

assets/base.css

Oliviammarcello · 2021-08-11T13:49:25Z

Looks great!

Just a few fine details to fix:

We just got feedback that the "share" label should now be an input field to allow merchants to change it. Sorry for the last-minute change. Screenshot
Add the hover styles that Sofia mentioned above
The spacing on the blog post page is still a bit large. Screenshot 1 | Screenshot 2
Can the dropdown for share have the same animation as the navigation dropdowns? Video.

svinkle · 2021-08-11T14:08:06Z

Three things from an accessibility perspective:

For the JS + fallback env only, apply the disclosure pattern as described in [Global] Details/summary pattern conveys incorrect semantic meaning #78.
When the close control is activated, shift focus back to the "Share" control. Otherwise, focus is lost and in some env, shifts to the top of the DOM.
Live regions have the best support across environments when content is dynamically injected. The aria-hidden attr technique is not supported in Windows. Since the live content is visible and persists, remove the aria-hidden functionality. Instead, dynamically insert the text content via node.textContent. Clear the content container when the disclosure context is dismissed.

svinkle · 2021-08-11T17:05:40Z

@ludoboludo Is there a new demo link where I can test the changes?

ludoboludo · 2021-08-11T17:34:41Z

@ludoboludo Is there a new demo link where I can test the changes?

Yes @svinkle, sorry for the delay. Links are updated in the PR description. (storefront)

sofiamatulis

Thanks for all the updates 👍 Did a review on the code and then I can do a final overall pass

assets/base.css

assets/section-blog-post.css

svinkle · 2021-08-11T20:03:23Z

@ludoboludo Looking good. Still not sure about the focus management part. Can you confirm focus shifts back to the share control on close? I'm not seeing a focus ring, nor do I see the control in the console when I use my What has focus? bookmarklet.

ludoboludo · 2021-08-11T21:52:32Z

Can you confirm focus shifts back to the share control on close? I'm not seeing a focus ring, nor do I see the control in the console when I use my What has focus? bookmarklet.

Confirmed you were right! I was focusing the button instead of the summary element, should be good now 👌

sofiamatulis

Looks great :) Thanks for all the changes, Ludo 🎉

Added one small comment to update the motion-reduce to only be in the animateMenuOpen, but not in the icons since we dont have it on the other icons in the theme (for consistency and to follow the best practises)

sections/main-article.liquid

svinkle · 2021-08-12T17:09:05Z

@ludoboludo I'm unable to preview the newest change. The previous store link has an error.

ludoboludo · 2021-08-12T17:50:55Z

@ludoboludo I'm unable to preview the newest change. The previous store link has an error.

@svinkle hmm odd. Something must have gone wrong when I copied the preview link. Should be good now.

svinkle · 2021-08-12T18:29:07Z

Same issue: Theme cannot be previewed because it's missing one of these required files: layout/theme.liquid, config/settings_schema.json

gretchen-willenberg · 2021-08-12T18:39:14Z

Low priority side revision: replace cannot with can't – cannot is a tell that content design didn't polish this message.
Theme can't be previewed because it's missing one of these required files: layout/theme.liquid, config/settings_schema.json

And, dumb question, but is this 1 file? The comma makes it seem like 2/multiple can be included so:
Theme can't be previewed because it's missing required file(s):

ludoboludo · 2021-08-12T19:52:11Z

Theme can't be previewed because it's missing one of these required files:

@gretchen-willenberg this would be a change on the platform level. It's not part of the theme, I could do some digging if necessary to see which team could tackle it.

gretchen-willenberg · 2021-08-12T19:59:20Z

Sorry, less a priority than Dawn, more of a nice-to-have improvement. Don't add to your workload now!

LucasLacerdaUX

Looks good to me. Just left a minor adjustment but everything should be good :D

assets/share.js

Co-authored-by: Lucas Lacerda <37168033+LucasLacerdaUX@users.noreply.github.com>

ludoboludo · 2021-08-17T19:04:09Z

Followed up on your suggestion @LucasLacerdaUX so it cancelled your approval. I'd need it again along with @sofiamatulis as well 🙂

LucasLacerdaUX

LGTM.

💪 🐶 👍

sofiamatulis

🚢

ludoboludo added 4 commits August 5, 2021 16:43

first draft

453f625

start working on the share pop up styles

faf9041

move around the copy message and add close button

337b732

add close button event

a8768b4

sofiamatulis self-assigned this Aug 9, 2021

ludoboludo commented Aug 9, 2021

View reviewed changes

remove unused code

e410d3d

ludoboludo requested review from Oliviammarcello and svinkle August 9, 2021 18:48

sofiamatulis reviewed Aug 9, 2021

View reviewed changes

LucasLacerdaUX self-requested a review August 9, 2021 19:02

LucasLacerdaUX reviewed Aug 9, 2021

View reviewed changes

sections/main-article.liquid Show resolved Hide resolved

ludoboludo added 4 commits August 9, 2021 17:49

address reviewer's comments

128a900

added accessibility attribute + design tweaks

e3b42c6

address design comments

90583de

underline removal when active and reduced width of the details

c0dad44

ludoboludo requested review from sofiamatulis and LucasLacerdaUX August 10, 2021 17:21

fix the details' width

1550ea8

sofiamatulis reviewed Aug 10, 2021

View reviewed changes

add overflow property

6f1491f

accessibility changes and animation/hover

a2f1730

spacing of share button on article page

a136194

Oliviammarcello previously approved these changes Aug 11, 2021

View reviewed changes

ludoboludo requested a review from sofiamatulis August 11, 2021 19:22

sofiamatulis reviewed Aug 11, 2021

View reviewed changes

fix element to focus on

122bcd9

ludoboludo dismissed Oliviammarcello’s stale review via 122bcd9 August 11, 2021 21:53

hide btn when no js and add motion reduce

52792f7

ludoboludo requested a review from sofiamatulis August 12, 2021 14:28

sofiamatulis previously approved these changes Aug 12, 2021

View reviewed changes

sections/main-article.liquid Outdated Show resolved Hide resolved

remove motion reduce from share icons

b9bc887

ludoboludo dismissed sofiamatulis’s stale review via b9bc887 August 12, 2021 15:15

svinkle previously approved these changes Aug 12, 2021

View reviewed changes

LucasLacerdaUX previously approved these changes Aug 17, 2021

View reviewed changes

assets/share.js Outdated Show resolved Hide resolved

ludoboludo dismissed stale reviews from LucasLacerdaUX and svinkle via 0881918 August 17, 2021 18:51

Update assets/share.js

0881918

Co-authored-by: Lucas Lacerda <37168033+LucasLacerdaUX@users.noreply.github.com>

LucasLacerdaUX approved these changes Aug 17, 2021

View reviewed changes

sofiamatulis approved these changes Aug 17, 2021

View reviewed changes

ludoboludo merged commit 789c3d9 into main Aug 17, 2021

ludoboludo deleted the share-button-update branch August 17, 2021 21:21

LucasLacerdaUX mentioned this pull request Aug 17, 2021

Add featured product section #261

Merged

5 tasks

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024

Share button UI update (Shopify#349)

72e6226

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Share button UI update #349

Share button UI update #349

ludoboludo commented Aug 9, 2021 •

edited

Loading

ludoboludo Aug 9, 2021

Oliviammarcello Aug 9, 2021

gretchen-willenberg Aug 9, 2021

ludoboludo Aug 10, 2021

sofiamatulis left a comment

sofiamatulis Aug 9, 2021

ludoboludo Aug 10, 2021

ludoboludo Aug 10, 2021

sofiamatulis Aug 10, 2021

sofiamatulis Aug 10, 2021

ludoboludo Aug 10, 2021

Oliviammarcello commented Aug 9, 2021 •

edited by ludoboludo

Loading

sofiamatulis left a comment

sofiamatulis Aug 10, 2021

ludoboludo Aug 11, 2021

Oliviammarcello commented Aug 11, 2021 •

edited by ludoboludo

Loading

svinkle commented Aug 11, 2021

svinkle commented Aug 11, 2021

ludoboludo commented Aug 11, 2021 •

edited

Loading

sofiamatulis left a comment

svinkle commented Aug 11, 2021

ludoboludo commented Aug 11, 2021

sofiamatulis left a comment

svinkle commented Aug 12, 2021

ludoboludo commented Aug 12, 2021

svinkle commented Aug 12, 2021

gretchen-willenberg commented Aug 12, 2021

ludoboludo commented Aug 12, 2021

gretchen-willenberg commented Aug 12, 2021

LucasLacerdaUX left a comment

ludoboludo commented Aug 17, 2021

LucasLacerdaUX left a comment •

edited

Loading

sofiamatulis left a comment

Share button UI update #349

Share button UI update #349

Conversation

ludoboludo commented Aug 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sofiamatulis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oliviammarcello commented Aug 9, 2021 • edited by ludoboludo Loading

sofiamatulis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oliviammarcello commented Aug 11, 2021 • edited by ludoboludo Loading

svinkle commented Aug 11, 2021

svinkle commented Aug 11, 2021

ludoboludo commented Aug 11, 2021 • edited Loading

sofiamatulis left a comment

Choose a reason for hiding this comment

svinkle commented Aug 11, 2021

ludoboludo commented Aug 11, 2021

sofiamatulis left a comment

Choose a reason for hiding this comment

svinkle commented Aug 12, 2021

ludoboludo commented Aug 12, 2021

svinkle commented Aug 12, 2021

gretchen-willenberg commented Aug 12, 2021

ludoboludo commented Aug 12, 2021

gretchen-willenberg commented Aug 12, 2021

LucasLacerdaUX left a comment

Choose a reason for hiding this comment

ludoboludo commented Aug 17, 2021

LucasLacerdaUX left a comment • edited Loading

Choose a reason for hiding this comment

sofiamatulis left a comment

Choose a reason for hiding this comment

ludoboludo commented Aug 9, 2021 •

edited

Loading

Oliviammarcello commented Aug 9, 2021 •

edited by ludoboludo

Loading

Oliviammarcello commented Aug 11, 2021 •

edited by ludoboludo

Loading

ludoboludo commented Aug 11, 2021 •

edited

Loading

LucasLacerdaUX left a comment •

edited

Loading