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

🖍 Added shopping tag flip on too far position to the right #37195

Merged
merged 52 commits into from
Feb 2, 2022

Conversation

jshamble
Copy link
Contributor

@jshamble jshamble commented Dec 13, 2021

Added the ability to have a shopping tag auto-change it's pill position to the left of the dot if it is too far to the right of the screen.

Also flips the dot to align with the correct spot for RTL languages.

Fixes #37028

Normal LTR Flipped LTR
Screen Shot 2021-12-13 at 8 24 40 PM Screen Shot 2021-12-13 at 8 32 00 PM
Normal RTL Flipped RTL
Screen Shot 2022-01-06 at 10 01 38 PM Screen Shot 2022-01-06 at 10 01 56 PM

@amp-owners-bot
Copy link

amp-owners-bot bot commented Dec 13, 2021

Hey @gmajoulet! These files were changed:

extensions/amp-story-shopping/0.1/amp-story-shopping-tag.css
extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js
extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-tag.js

@jshamble jshamble changed the title 🖍 Added shopping tag flip on specific position to the right 🖍 Added shopping tag flip on too far position to the right Dec 13, 2021
Copy link
Contributor

@coreymasanto coreymasanto left a comment

Choose a reason for hiding this comment

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

Could you add a screenshot of a story that includes both normal and flipped shopping tags?

@jshamble jshamble removed the request for review from processprocess December 14, 2021 01:00
@processprocess
Copy link
Contributor

processprocess commented Jan 13, 2022

There's another piece to this concept that needs to be worked into this PR.
The element needs to be transformed so that the indicator dot is in the same place as it was before the flip. This ensures that the tag will be on screen.

Screen Shot 2022-01-13 at 10 15 01 AM

An approach:
If the element is off-screen on the right:
translateX by -100% + the size of the dot.
If off-screen on the left, a flipped version of the same equation can be used.

We will need to make this check on window resize:

Jan-14-2022.10-46-01.mp4

@processprocess
Copy link
Contributor

We need to hide the button until the calculation is made and the element is flipped. Otherwise we see it flash on one side then flip to the other:

This could be accomplished by setting opacity to 0 initially and setting it to 1 after the check and positioning is done.

Also the margin between the button and pill is gone.
Please bring that back.

Jan-14-2022.11-39-53.mp4

Copy link
Contributor

@processprocess processprocess left a comment

Choose a reason for hiding this comment

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

Looks awesome!

@processprocess
Copy link
Contributor

The margins are broken in RTL. Please take a look into this:

Screen Shot 2022-02-02 at 1 47 52 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[amp story shopping] Shopping tag positioning on edge of page
6 participants