Skip to content

Conversation

@alex-page
Copy link
Member

WHY are these changes introduced?

Fixes minor visual bug with VideoThumbnail

WHAT is this pull request doing?

Adds a top left and right border-radius to the image in the VideoThumbnail

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Look at the VideoThumbnail example in Storybook

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@alex-page alex-page requested a review from chloerice April 15, 2020 14:21
@alex-page alex-page self-assigned this Apr 15, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Apr 15, 2020

🟢 This pull request modifies 2 files and might impact 1 other files.

Details:
All files potentially affected (total: 1)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/MediaCard/MediaCard.scss (total: 1)

Files potentially affected (total: 1)

@dfmcphee
Copy link
Contributor

dfmcphee commented Apr 15, 2020

This looks good to me, but I think we may need a fix for the image media cards as well.

Screen Shot 2020-04-15 at 10 32 45 AM

I think adding this to the 'MediaContainer' on small screens would do it

    border-top-left-radius: border-radius();
    border-top-right-radius: border-radius();
    overflow: hidden;

@kyledurand
Copy link
Member

This looks good for the portrait card but not so much on landscape

image

@alex-page
Copy link
Member Author

alex-page commented Apr 15, 2020

I have moved the change to .MediaContainer and fixed up the responsive problems.

Video thumbnail desktop
Screen Shot 2020-04-15 at 11 19 06 AM

Video thumbnail mobile
Screen Shot 2020-04-15 at 11 19 16 AM

Media thumbnail portrait mobile
Screen Shot 2020-04-15 at 11 19 36 AM

Media thumbnail portrait desktop
Screen Shot 2020-04-15 at 11 19 42 AM

@alex-page alex-page changed the title [VideoThumbnail] Add top left and right border-radius [MediaCard] Add top left and right border-radius Apr 15, 2020
@alex-page alex-page changed the title [MediaCard] Add top left and right border-radius [MediaCard] Add border-radius when necessary Apr 15, 2020
Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

👍

@alex-page alex-page merged commit 2e13fe3 into master Apr 15, 2020
@alex-page alex-page deleted the videothumbnail-border-radius branch April 15, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants