Remove white space mobile view for product page #655

sofiamatulis · 2021-09-20T15:05:56Z

Why are these changes introduced?

Fixes #499 .

What approach did you take?

Added display: none; when the visibility is set to hidden

Other considerations

Demo links

Test it on the product Pleated Heel Boot off white on mobile (by resizing the screen or viewing it in the editor in the mobile view)

You should still see the view in your space when you are using a device: https://screenshot.click/20-03-zhf48-0umjv.png

Editor

Checklist

kmeleta · 2021-09-20T18:11:29Z

assets/component-product-model.css

@@ -11,6 +11,7 @@

 .product__xr-button[data-shopify-xr-hidden] {
  visibility: hidden;


I’m not sure why visibility was chosen over display initially, but the button still appears as expected on mobile so hiding via display seems safe. In any case, I imagine we don’t also need the visibility style here now do we?

Good point! Removed

ludoboludo

Seems to work as expected. 👍

kmeleta

Tested on desktop browsers and mobile device 👍

LucasLacerdaUX · 2021-09-20T18:56:06Z

assets/component-product-model.css

@@ -10,7 +10,7 @@
 }

 .product__xr-button[data-shopify-xr-hidden] {
-  visibility: hidden;
+  display: none;


cc @melissaperreault Can I get your 👍 on this, just to confirm that we changed our approach from previous product media decisions?

For context: The visibility:hidden was intentionally set to avoid content jumping while the page is loading. This follows our approach on every previous theme that had 3D media implemented. We decided (at the time) that having some extra whitespace was a smaller issue than this page jumping:

This can be an issue on slower connections (simulate Slow 3G connection on Chrome Developer Tools to better understand this), where the page might still be loading and the user starts interacting with it, then the View in your space button appears and pushes the content - possibly making the user click a button by accident.

At first glance I am concerned by this page jump happening without user interaction and can lead to a poor UX. It is something we debated on our internal implementation of our Debut theme. (https://github.com/Shopify/debut/pull/1166)

I would like to bring @tylerrowsell and @sainihas to the conversation as this is affecting a global feature experience and want to bring the right people in the conversation.

I'd like to check and sync about other alternatives we could have.

cc. @nicklepine

From the technical side of things, one thing that comes to mind as a possible solution to explore... We could detect user agents server side and then make that info available in a liquid drop. It would require a list of all mobile user agents but perhaps could be a starting point. Then we could use progressive enhancements in js to further check ones that we don't detect via user agent.

I am not confident in how reliable that would be though, for that reason I would suggest we consider an alternative button/layout for "view in your space".

I'll defer to Sai on this one about potential alternative for design.

Since the main goal of this PR is to avoid the extra space when the merchant is viewing the product page with the 3D button on mobile in the editor mode, I have added the display: none only when in the editor view. This ensures that we keep the visibility hidden on the actual devices and this way the user wont experience any jumping

Here is the commit: 87d531a

ludoboludo

Tested on chrome and safari and seems to do the job 👌

melissaperreault

Glad we found a clean approach for both merchants and buyers experience! 🚢

sofiamatulis · 2021-09-22T17:54:35Z

When reviewing this PR, Ken noted that there was a bug on master (not related to this PR)

This whitespace above the arrows is a bug and should be removed. (aka replace visibility:hidden with display:none). The space is there because there are two buttons on mobile and there should only be only one (ipad and mobile differ where the button is situated)
The second whitespace, below the arrows, is where the button will actually be shown. That whitespace should be kept (outside the editor)
On the editor, you shouldn’t have any whitespace (add display: none to like it's already done)

Expectations:

Mobile view on your laptop: https://screenshot.click/22-37-gvlo4-j5q57.png
Editor mobile view: https://screenshot.click/22-38-vmdjs-0hj6n.png
On your actual phone: https://screenshot.click/22-53-aosiu-9mt0q.png
On your actual ipad: https://screenshot.click/22-54-a267k-axquy.png

@melissaperreault @ludoboludo I have re-requested your reviews

LucasLacerdaUX

Looks good to me.

I'm not so sure about the decision to hide the button instead of force-displaying it on the Editor from a UX perspective, so I just left some comments below.

LucasLacerdaUX · 2021-09-22T17:55:14Z

assets/product-model.js

@@ -54,5 +54,8 @@ window.ProductModel = {
 };

 window.addEventListener('DOMContentLoaded', () => { 
+  if (Shopify.designMode) {
+    document.querySelectorAll("[data-shopify-xr-hidden]").forEach(element => element.classList.add('hidden'));


For this Editor Mobile view, I feel that second button could be displayed inside the editor instead of adding display: none to it. This would allow the merchant to see and tweak its colors.

The weird part about it, though, is that merchants wouldn't be able to launch the actual XR (so the button would act as disabled). AFAIK, the shopify-xr script does not work in Editor mode.

I’m also okay with this current solution as it matches the decision we previously made (to have a whitespace).

I had a conversation with Ludo on this and I feel like it makes sense to keep it hidden on the browser since this can only be used on the actual device (and it wouldnt work). This way we are also consistent with other themes.

@melissaperreault Let me know your thoughts - we can always create a follow up enhancement issue

Would it make sense to show the button and then use some sort of notification or alert to explain to the merchant that on a mobile device this would activate "view in your space" ?

But then not all mobile device support the view in your space thing right ? So we also need that mentioned.
Unless it shows on any device and takes you to the app store to use a compatible app to view it 🤷

@LucasLacerdaUX is bringing a good point.

We'll need to keep monitoring merchant's experience when they want to validate how the feature will work/display for them and their customers when editing their theme. If they don't see any button in the editor view, regardless if it is compatible or not, they might think it is a bug and could contact support advisor, etc. I am surprised we don't have any insights on that experience yet, isn't it something that was happening on Debut and Narrative already?

On the other side, we don't want to show an invalid layout to merchants as they would again perceive it as a bug because they wouldn't be able to compare what is the reality for the same reasons.

A potential missed opportunity is to have a fallback CTA label/tooltip in place that informs buyers there is a 3D model available and that they should consider viewing this product on the proper device to have the right experience. cc. @bldelgado Do you recall if this was explored? 🤔

For the short term, I don't mind having this fix until we find the right experience, but in the meantime, I would hope that merchants help docs could inform merchants that they could never see the View in your space button in the editor mode. Thoughts? (I am not sure where in the help docs this information could live, at the product media type level but perhaps also at the theme customization detail level?)

This would allow the merchant to see and tweak its colors

I don't think this is the biggest concern as the theme color scheme doesn't allow you to tweak granular colors like that.

But then not all mobile device support the view in your space thing right ? So we also need that mentioned.
Unless it shows on any device and takes you to the app store to use a compatible app to view it

Most Android/Apple devices will support it. It does not require an app and uses the native functionality of iOS Quicklook or Android Scene Viewer if the device is new enough.

kmeleta

Meli and Ludo should weigh in, but the latest changes now align with my expectations 👍 Thanks!

LucasLacerdaUX

The alignment fix broke the display: none we just added to one of the buttons and re-introduced the whitespace above the slider arrows.

LucasLacerdaUX · 2021-09-23T13:57:03Z

assets/component-product-model.css

@@ -3,6 +3,7 @@
  color: rgb(var(--color-foreground));
  margin: 1rem auto;
  box-shadow: none;
+  display: flex;


This is overriding the display: none we added below:

Can you please verify if we need the .button before the .product__xr-button? I don't remember why I added it in the first place, maybe we had some conflict / specificity issue before.

EDIT: Now I remember why I added this .button. It was being used to override the hover style, so I believe you don't need it on the main selector and can update it to look like that (and ignore the suggestion in my next comment)

It still seems to hide it properly with display: none !important applied to it from the hidden class. From here:

So maybe we just don't need that other display none 🤔

That JS solution is only applied on Design Mode. The issue I mentioned above, related to the CSS property, is applied both inside and outside the Editor.

Before / After (outside the Theme Editor)

I can see needing the class button because we have a modifier class right after 🤔 button--full-width

Good catch on the flex. I am reverting that and will try to align it on the ipad breakdown differently

Ah gotcha, I was focusing on the Editor 🤦 but I see it on the "live" version 👍 good catch.

Oh, I wasn't suggesting to remove the button class from the element. Sorry, that was a bit confusing on my first comment.

My suggestion was to reduce the specificity from the CSS selector (except on :hover):

If not possible, then you can apply the solution in my next comment below - to increase the specificity on the selector inside the media query.

I apologize - I misunderstood your comment 😅 I removed the specificity on product__xr-button and kept the display flex and it seems to work on all of the scenarios: 1) mobile screens on desktop 2) editor view on mobile 3) actual devices on mobile and tablet

assets/component-product-model.css

melissaperreault

Good for this PR to 🚢 but observed we could make an extra effort to optimizing vertical spacing in another PR 🙏

In this example, in blue, I am confident we can reduce some pixels between the CTA and the next block below it but we'll need to check various scenarios. i.e We already identified that the Title block would need a little update because the top margin is way to large right now.

ludoboludo

Should we still see the space when doing a mobile view in our browser ? 🤔 We keep it there so there isn't any layout shift right ? screenshot

Just getting confused about where and when we should see the white gap 😅

sofiamatulis · 2021-09-24T14:18:11Z

Should we still see the space when doing a mobile view in our browser ? 🤔 We keep it there so there isn't any layout shift right ? screenshot

Just getting confused about where and when we should see the white gap 😅

@ludoboludo We do keep it there on mobile on browser :)

All the scenarios are here: #655 (comment)

Remove white space mobile view for product page

f444f26

sofiamatulis added the Category: Bug Something isn't working label Sep 20, 2021

kmeleta self-requested a review September 20, 2021 18:06

kmeleta reviewed Sep 20, 2021

View reviewed changes

Remove visibility hidden

75a15a0

sofiamatulis requested a review from kmeleta September 20, 2021 18:24

sofiamatulis assigned kmeleta Sep 20, 2021

ludoboludo self-requested a review September 20, 2021 18:39

ludoboludo previously approved these changes Sep 20, 2021

View reviewed changes

kmeleta previously approved these changes Sep 20, 2021

View reviewed changes

LucasLacerdaUX self-requested a review September 20, 2021 18:43

LucasLacerdaUX reviewed Sep 20, 2021

View reviewed changes

sofiamatulis requested a review from melissaperreault September 20, 2021 19:14

ludoboludo self-requested a review September 21, 2021 14:00

kmeleta self-requested a review September 21, 2021 14:41

Add display none when editor mode

87d531a

sofiamatulis dismissed stale reviews from kmeleta and ludoboludo via 87d531a September 21, 2021 18:49

sofiamatulis requested a review from LucasLacerdaUX September 21, 2021 18:55

sofiamatulis assigned ludoboludo Sep 21, 2021

ludoboludo previously approved these changes Sep 21, 2021

View reviewed changes

melissaperreault previously approved these changes Sep 22, 2021

View reviewed changes

Update spacing for mobile

73e2c44

sofiamatulis dismissed stale reviews from melissaperreault and ludoboludo via 73e2c44 September 22, 2021 17:42

sofiamatulis requested review from ludoboludo and melissaperreault September 22, 2021 17:54

LucasLacerdaUX previously approved these changes Sep 22, 2021

View reviewed changes

kmeleta previously approved these changes Sep 22, 2021

View reviewed changes

fix align

9e5da6c

sofiamatulis dismissed stale reviews from kmeleta and LucasLacerdaUX via 9e5da6c September 22, 2021 18:20

sofiamatulis requested review from kmeleta and LucasLacerdaUX September 22, 2021 18:20

ludoboludo previously approved these changes Sep 22, 2021

View reviewed changes

LucasLacerdaUX reviewed Sep 23, 2021

View reviewed changes

Revert

cb84008

sofiamatulis dismissed ludoboludo’s stale review via cb84008 September 23, 2021 14:43

Update specifity class;

f4843fe

sofiamatulis requested review from tylerrowsell, LucasLacerdaUX and ludoboludo September 23, 2021 15:55

melissaperreault approved these changes Sep 23, 2021

View reviewed changes

ludoboludo reviewed Sep 24, 2021

View reviewed changes

ludoboludo approved these changes Sep 24, 2021

View reviewed changes

LucasLacerdaUX approved these changes Sep 24, 2021

View reviewed changes

sofiamatulis merged commit ce3459c into main Sep 24, 2021

sofiamatulis deleted the white-space branch September 24, 2021 19:53

sofiamatulis mentioned this pull request Sep 24, 2021

Optimize spacing on the product page #687

Closed

KaichenWang mentioned this pull request Sep 29, 2021

Version bump to 2.2.0 #712

Merged

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

Remove white space mobile view for product page (Shopify#655)

f13aaec

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove white space mobile view for product page #655

Remove white space mobile view for product page #655

sofiamatulis commented Sep 20, 2021 •

edited

Loading

kmeleta Sep 20, 2021

sofiamatulis Sep 20, 2021

ludoboludo left a comment

kmeleta left a comment

LucasLacerdaUX Sep 20, 2021

melissaperreault Sep 21, 2021

tylerrowsell Sep 21, 2021

sofiamatulis Sep 21, 2021 •

edited

Loading

sofiamatulis Sep 21, 2021

ludoboludo left a comment •

edited

Loading

melissaperreault left a comment

sofiamatulis commented Sep 22, 2021 •

edited

Loading

LucasLacerdaUX left a comment

LucasLacerdaUX Sep 22, 2021

sofiamatulis Sep 22, 2021 •

edited

Loading

tylerrowsell Sep 22, 2021

ludoboludo Sep 22, 2021 •

edited

Loading

melissaperreault Sep 23, 2021 •

edited

Loading

tylerrowsell Sep 23, 2021

kmeleta left a comment

LucasLacerdaUX left a comment

LucasLacerdaUX Sep 23, 2021 •

edited

Loading

ludoboludo Sep 23, 2021

LucasLacerdaUX Sep 23, 2021

sofiamatulis Sep 23, 2021

ludoboludo Sep 23, 2021

LucasLacerdaUX Sep 23, 2021 •

edited

Loading

sofiamatulis Sep 23, 2021

melissaperreault left a comment

ludoboludo left a comment

sofiamatulis commented Sep 24, 2021

		@@ -11,6 +11,7 @@

		.product__xr-button[data-shopify-xr-hidden] {
		visibility: hidden;

Remove white space mobile view for product page #655

Remove white space mobile view for product page #655

Conversation

sofiamatulis commented Sep 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludoboludo left a comment

Choose a reason for hiding this comment

kmeleta 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

sofiamatulis Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludoboludo left a comment • edited Loading

Choose a reason for hiding this comment

melissaperreault left a comment

Choose a reason for hiding this comment

sofiamatulis commented Sep 22, 2021 • edited Loading

LucasLacerdaUX left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sofiamatulis Sep 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludoboludo Sep 22, 2021 • edited Loading

Choose a reason for hiding this comment

melissaperreault Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmeleta left a comment

Choose a reason for hiding this comment

LucasLacerdaUX left a comment

Choose a reason for hiding this comment

LucasLacerdaUX Sep 23, 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

Choose a reason for hiding this comment

LucasLacerdaUX Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melissaperreault left a comment

Choose a reason for hiding this comment

ludoboludo left a comment

Choose a reason for hiding this comment

sofiamatulis commented Sep 24, 2021

sofiamatulis commented Sep 20, 2021 •

edited

Loading

sofiamatulis Sep 21, 2021 •

edited

Loading

ludoboludo left a comment •

edited

Loading

sofiamatulis commented Sep 22, 2021 •

edited

Loading

sofiamatulis Sep 22, 2021 •

edited

Loading

ludoboludo Sep 22, 2021 •

edited

Loading

melissaperreault Sep 23, 2021 •

edited

Loading

LucasLacerdaUX Sep 23, 2021 •

edited

Loading

LucasLacerdaUX Sep 23, 2021 •

edited

Loading