-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[VideoThumbmail] Improve VideoThumbnail play button #7319
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
Conversation
size-limit report 📦
|
c91e5d0 to
a1df087
Compare
|
/snapit |
|
🫰✨ Thanks @sophschneider! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
| <div | ||
| className={styles.Thumbnail} | ||
| style={{backgroundImage: `url(${thumbnailUrl})`}} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6fec50b to
ba10689
Compare
| width: 100%; | ||
| height: 100%; | ||
| padding-bottom: auto; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish there was a tool to find unused scss class names. Could be impactful across multiple repos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually looking into something like that this morning :D Definitely possible and would be so useful
ba10689 to
ab2ff76
Compare
| onTouchStart={onBeforeStartPlaying} | ||
| > | ||
| <img className={styles.PlayIcon} src={PlayIcon} alt="" /> | ||
| {timeStampMarkup} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this inside since it changes opacity on hover and the button is the whole thumbnail
ab2ff76 to
d44323e
Compare
|
/snapit |
|
🫰✨ Thanks @sophschneider! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
mrcthms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved as long as we don't need the updates to the Timestamp that occur on hover on focus as well
| <span className={styles.PlayIcon}> | ||
| <Icon source={PlayMinor} /> | ||
| </span> | ||
| <Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a Text component now?! 🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yesssssss. Spread the word @mrcthms 🚀
https://polaris.shopify.com/whats-new/version-10-typography
5bef0ea to
71dbd69
Compare
71dbd69 to
3a96c92
Compare
3a96c92 to
66aa459
Compare
| } | ||
|
|
||
| &:hover .Timestamp { | ||
| background-color: rgba(0, 0, 0, 0.8); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these custom colours ok? design had black with this opacity and no token was suitable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are okay for now. We can find/fix them up later.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/[email protected] ### Minor Changes - [#7216](#7216) [`fbf2f8832`](fbf2f88) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add migration to replace static mixins with declarations ### Patch Changes - [#7328](#7328) [`b31f51f25`](b31f51f) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add namespace support to the `replace-static-breakpoint-mixins` migration ## @shopify/[email protected] ### Minor Changes - [#7360](#7360) [`f7140123d`](f714012) Thanks [@mrcthms](https://github.com/mrcthms)! - Update `IndexTable` in sortable mode to fix visual bugs and include label Tooltips ### Patch Changes - [#7361](#7361) [`b1b576403`](b1b5764) Thanks [@kyledurand](https://github.com/kyledurand)! - Use state callback in page actions - [#7319](#7319) [`4b95fdc64`](4b95fdc) Thanks [@qt314](https://github.com/qt314)! - Update the `VideoThumbnail` play button user experience ## @shopify/[email protected] ### Patch Changes - Updated dependencies \[[`fbf2f8832`](fbf2f88), [`b31f51f25`](b31f51f)]: - @shopify/[email protected] ## [email protected] ### Minor Changes - [#7254](#7254) [`61cf086ed`](61cf086) Thanks [@nickpresta](https://github.com/nickpresta)! - Added ability to collapse props that have been expanded. ### Patch Changes - [#7305](#7305) [`b0445cf9b`](b0445cf) Thanks [@selenehinkley](https://github.com/selenehinkley)! - Added "Getting started" section - Updated dependencies \[[`f7140123d`](f714012), [`b1b576403`](b1b5764), [`4b95fdc64`](4b95fdc)]: - @shopify/[email protected] ## [email protected] ### Patch Changes - Updated dependencies \[[`f7140123d`](f714012), [`b1b576403`](b1b5764), [`4b95fdc64`](4b95fdc)]: - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>


WHY are these changes introduced?
Fixes #6864 #6859 https://github.com/Shopify/shopify/issues/355172
Visual update to the VideoThumbnail play button redesigned by the Admin Quality Crew team
https://5d559397bae39100201eedc1-butsvtcrne.chromatic.com/?path=/story/all-components-videothumbnail--default
https://admin.web.web-p6n3.sophie-schneider.us.spin.dev/store/shop1/marketing
WHAT is this pull request doing?
Before: existing
VideoThumbnailAfter: UX updates
How to 🎩
Resize the windows and test the hover states in the following:
🎩 checklist
Voice over