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

🐛 [BUG] - Pillar Image added whitespace when as="picture" attribute is used. #911

Open
joshvogel17 opened this issue Jan 28, 2025 · 3 comments
Assignees
Labels
brand bug Something isn't working

Comments

@joshvogel17
Copy link

joshvogel17 commented Jan 28, 2025

Describe the bug

Found while working on: https://github.com/github/marketing-platform-services/issues/3793
Relevant PR: https://github.com/github/github/pull/359729

When adding a Pillar.Image and using the as="picture" the border on the bottom of the image was not rounded. Inspecting the page in the developers tools showed there to be some additional whitespace at the bottom of the image causing the issue. As a workaround, adding the style vertical-align: bottom to Pillar.Image seemed to resolve the issue.

Image where as="picture" property is set
Image

Image where default as property is set

Image

Reproduction steps

1. Create Pillar with Pillar.Image
2. Set `as="picture"` property to `Pillar.Image`
3. View Pillar. Notice that the border is not rounded on the bottom.

Expected behavior

Pillar.Image with as="picture" property set will still have a rounded border on the bottom.

Screenshots

Browsers

Chrome

OS

Mac

@joshvogel17 joshvogel17 added the bug Something isn't working label Jan 28, 2025
@seangolob
Copy link
Collaborator

Hi PB team!

I think I have a similar issue except with the card component. When I try to leverage as="picture" on the Card.Image, I am seeing a missing rounded border at the bottom.

Before (no picture tag):

Image

After (with picture tag):

Image

I believe the issue is the picture tag in this case has display: inline-block style on it which is causing 4px of whitespace under the image. I have a workaround in place to avoid this issue so I am by no means blocked.

Inspect view (with picture tag):

Image

@rezrah rezrah self-assigned this Feb 10, 2025
@rezrah rezrah added the brand label Feb 10, 2025
@seangolob
Copy link
Collaborator

Hi there! I am experiencing a similar issue to these with the "River" element when I try to leverage the picture tag. The picture element is missing a height: 100% style so it doesn't consistently fill the height of its container. I believe we will need to adjust https://github.com/primer/brand/blob/main/packages/react/src/river/river-shared.module.css#L17 to add support for making the picture tag height: 100%.

This bug can be experienced here. However, you must be logged in to view the behavior.

Image

I have a workaround in place so this doesn't block me at all.

@seangolob
Copy link
Collaborator

seangolob commented Feb 18, 2025

@rezrah I've been piling into this issue a bit recently. I can easily split out the 3 separate comments into separate issues if that makes things easier for you or whomever works on this. My thought process is that these changes are all related to the picture tag so it easier to have it one spot. 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brand bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants