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

developers.google.com - layout is messed up #1461

Closed
simevidas opened this issue Jul 24, 2015 · 13 comments
Closed

developers.google.com - layout is messed up #1461

simevidas opened this issue Jul 24, 2015 · 13 comments

Comments

@simevidas
Copy link

URL: https://developers.google.com/web/updates/2015/07/23/devtools-digest-film-strip-and-a-new-home-for-throttling
Browser / Version: Firefox 39.0
Operating System: Windows 10 (Insider preview)
Problem type: Layout is messed up

Steps to Reproduce

  1. Navigate to: https://developers.google.com/web/updates/2015/07/23/devtools-digest-film-strip-and-a-new-home-for-throttling

Expected Behavior:

The embedded elements (images, videos, iframes) should respect their container’s width (which is 714px).

Actual Behavior:

Instead, they appear too large and make the entire layout too wide, overflowing the viewport.

@shospodarets
Copy link

Simplified demo: http://codepen.io/malyw/pen/zGmzbq?editors=110
Problem:
"max-width: 100%;" for image/video works in a different way inside a blocks with
"display: block;" / "display: flex;" rules.

@simevidas
Copy link
Author

@malyw Instead of width:auto; max-width:100%, using just width:100% seems to take care of the issue. Demo: http://codepen.io/simevidas/pen/XbxVaM?editors=110

Notice how the image and video remains responsive when you resize the viewport below 300px.

@karlcow
Copy link
Member

karlcow commented Jul 26, 2015

When the viewport is 360px

img, object, video {
    width: 100%;
    display: block;
    margin-top: 26px;
    margin-bottom: 26px;
}

.update-container img {
    width:auto;
    max-width:100%;
    display:inline-block;
    margin-top:0;
    margin-bottom:0
}

but when the viewport is for example 768px, we hit!

img, object, video {
    width: 100%;
    display: block;
    margin-top: 26px;
    margin-bottom: 26px;
}

.update-container img {
    width:auto;
    max-width:100%;
    display:inline-block;
    margin-top:0;
    margin-bottom:0
}

@media only screen and (min-width:620px) {
    …
    img, object, video {
        width: auto;
        max-width: 100%;
    }
}

In that case the width: 100% on video is dropped and Firefox hits the max-width: 100% in combination of width:auto; issue we have on many sites.
https://bugzilla.mozilla.org/show_bug.cgi?id=823483

If instead the site owners are doing:

.update-container img {
    display: inline-block;
    margin-top: 0px;
    margin-bottom: 0px;
}

@media only screen and (min-width:620px) {
    img, object, video {
        max-width: 100%;
    }
}

I need to check if it creates issues for smaller viewports.

@karlcow
Copy link
Member

karlcow commented Jul 27, 2015

@miketaylr do you know anyone working on developers.google.com ?

@simevidas
Copy link
Author

@karlcow There’s a discussion about this with Paul Bakaus on Twitter: https://twitter.com/pbakaus/status/624709026534682624. I think he can take care of this.

@karlcow
Copy link
Member

karlcow commented Jul 27, 2015

@pbakaus see above :) Thanks @simevidas

@pbakaus
Copy link

pbakaus commented Jul 27, 2015

I put in a very ugly temporary fix for Firefox (involving -moz-calc!), because I couldn't figure out a sensible fix.

width: 100% doesn't work because smaller images will be blown up to 100%. max-width on the container only works partially, as 100% seems to take the width of the body/viewport in Firefox. So I had to use calc to appropriate it.

This is a browser level issue I believe, and should be fixed in Firefox.

@karlcow
Copy link
Member

karlcow commented Jul 28, 2015

Thanks @pbakaus

Let's see if we can improve this a bit

There is no issue with smaller viewports on both blink and gecko

When I remove the width and max-width as below

.update-container img {
/*     width: auto;
    max-width: 100%; */
    display: inline-block;
    margin-top: 0;
    margin-bottom: 0
}

I have the correct behavior in Blink for small viewport and big viewport
And only correct behavior for Gecko in small viewport.

Then I added

@media only screen and (min-width:620px) {
    img, object, video {
        max-width: 100%;
    width: 100%;
    }
}

And it seems to be working both with Blink and Gecko (In this page).

@pbakaus is there a page where this suggestion blows up things?

@pbakaus
Copy link

pbakaus commented Jul 28, 2015

there are a quite a few pages. The thing is, said technique only works because you're using width: 100% (in the earlier example as well, as width: 100% comes from another CSS class then). But I don't want width: 100%. I want to be able to support images that are smaller than the default viewport.

@pbakaus
Copy link

pbakaus commented Jul 28, 2015

Here's a page for you to test against: https://developers.google.com/web/fundamentals/layouts/principles/menus-short

In this other part of the site, my additional styles from the Updates section aren't applied. In this case, the following logic controls the image:

Big viewport:

width: auto
max-width: 100%

Small viewport:

width: 100%

But here's the thing – that's crap. Try resizing that page until a certain level and the image gets blown up bigger than its actual size. Thus, that's simply not a solution (and is a bug on this page, in general).

@karlcow
Copy link
Member

karlcow commented Jul 28, 2015

(First of all I agree that there is a different interpretation in between Gecko/Blink 😄 I keep adding stuff to the bug which is happening in different circumstances. It's basically about shrink to fit and intrinsic width. The two rendering engines operate differently, but not sure if the behavior is specified in a spec.)

The issue on that page.
https://developers.google.com/web/fundamentals/layouts/principles/menus-short

I solved it with display: table; table-layout:fixed; on the figure element, which is a solution I tried on the initial page but could not find the right combination. Bummer. There's a bit of mystery in there. So it's no good. I'll try to find the right dose of silliness 🎯 because indeed the -moz-calc is ugly. Thanks a lot @pbakaus

@karlcow
Copy link
Member

karlcow commented Jul 28, 2015

@dholbert another one with the silly max-width issue on Gecko that I added to https://bugzilla.mozilla.org/show_bug.cgi?id=823483 but the main container <div class="update-container container"> has a display: flex.

@karlcow
Copy link
Member

karlcow commented Feb 10, 2016

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

No branches or pull requests

5 participants