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.

Sign up for GitHub

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

Remove white space mobile view for product page #655

Merged
merged 7 commits into from
Sep 24, 2021
Merged

Remove white space mobile view for product page #655

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f444f26
Remove white space mobile view for product page
sofiamatulis Sep 20, 2021
75a15a0
Remove visibility hidden
sofiamatulis Sep 20, 2021
87d531a
Add display none when editor mode
sofiamatulis Sep 21, 2021
73e2c44
Update spacing for mobile
sofiamatulis Sep 22, 2021
9e5da6c
fix align
sofiamatulis Sep 22, 2021
cb84008
Revert
sofiamatulis Sep 23, 2021
f4843fe
Update specifity class;
sofiamatulis Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/component-product-model.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

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

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Removed

display: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sofiamatulis sofiamatulis Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the commit: 87d531a

}

@media screen and (max-width: 749px) {
Expand Down