- 
                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/plugin-polaris@0.0.0-snapshot-release-20221003231640yarn add @shopify/polaris@0.0.0-snapshot-release-20221003231640 | 
    
| <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/plugin-polaris@0.0.0-snapshot-release-20221004161141yarn add @shopify/polaris@0.0.0-snapshot-release-20221004161141 | 
    
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/polaris-migrator@0.4.0 ### 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/polaris@10.6.0 ### 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/plugin-polaris@0.0.8 ### Patch Changes - Updated dependencies \[[`fbf2f8832`](fbf2f88), [`b31f51f25`](b31f51f)]: - @shopify/polaris-migrator@0.4.0 ## polaris.shopify.com@0.21.0 ### 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/polaris@10.6.0 ## polaris-for-figma@0.0.22 ### Patch Changes - Updated dependencies \[[`f7140123d`](f714012), [`b1b576403`](b1b5764), [`4b95fdc64`](4b95fdc)]: - @shopify/polaris@10.6.0 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