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

The `width` attribute #268

Closed
yoavweiss opened this Issue Jul 3, 2015 · 8 comments

Comments

Projects
None yet
6 participants
@yoavweiss
Member

yoavweiss commented Jul 3, 2015

During discussions about Client-Hints, where it makes sense to include the width attribute in the outgoing Width hint, since it can be useful to retrofit responsive images to legacy sites, @foolip suggested that it may make sense to include the same logic in srcset, and I think it makes a lot of sense.

Basically, the same logic applies. If sizes is missing and width is there and contains a non-% value, we should use the width value as the source-size-value (or add some "if sizes is not there, use width logic to the consumer of "parse a sizes attribute".

@zcorpan - WDYT?

@foolip

This comment has been minimized.

Show comment
Hide comment
@foolip

foolip Jul 3, 2015

My main concern here is that it would be nice if the computed source-size is the same as what is sent in the Width header. (Which is exactly the suggestion, of course.)

foolip commented Jul 3, 2015

My main concern here is that it would be nice if the computed source-size is the same as what is sent in the Width header. (Which is exactly the suggestion, of course.)

@joemcgill

This comment has been minimized.

Show comment
Hide comment
@joemcgill

joemcgill Jul 3, 2015

If I'm understanding the suggestion properly, i.e., use the width value as the default source-size-value instead of 100vw if the value is present (and a non-% value), then I'm mostly in agreement, and this is basically the logic we use to create a "smart default" for the sizes attribute in the WordPress plugin (see: ResponsiveImagesCG/wp-tevko-responsive-images#34).

The one suggestion I would propose, is to only use the width value when it is less than 100vw (if that's even possible). Otherwise, where someone has CSS for images set to max-width: 100% but has failed to set a sizes attribute, they could end up with a much larger download than needed.

joemcgill commented Jul 3, 2015

If I'm understanding the suggestion properly, i.e., use the width value as the default source-size-value instead of 100vw if the value is present (and a non-% value), then I'm mostly in agreement, and this is basically the logic we use to create a "smart default" for the sizes attribute in the WordPress plugin (see: ResponsiveImagesCG/wp-tevko-responsive-images#34).

The one suggestion I would propose, is to only use the width value when it is less than 100vw (if that's even possible). Otherwise, where someone has CSS for images set to max-width: 100% but has failed to set a sizes attribute, they could end up with a much larger download than needed.

@yoavweiss

This comment has been minimized.

Show comment
Hide comment
@yoavweiss

yoavweiss Jul 6, 2015

Member

Thanks @joemcgill :)

The change I'm proposing only relates to the width HTML attribute, so a max-width: 100% setting won't have any impact.

Member

yoavweiss commented Jul 6, 2015

Thanks @joemcgill :)

The change I'm proposing only relates to the width HTML attribute, so a max-width: 100% setting won't have any impact.

foolip added a commit to foolip/http-client-hints that referenced this issue Jul 6, 2015

@joemcgill

This comment has been minimized.

Show comment
Hide comment
@joemcgill

joemcgill Jul 6, 2015

Right, I understood, but was probably unclear in my description. I just meant that if an inline width attribute is used as a default when sizes is omitted (in the case of extending this logic to srcset calculations), then it may be preferential for the consumer to only use the width value when it's smaller than the viewport width.

joemcgill commented Jul 6, 2015

Right, I understood, but was probably unclear in my description. I just meant that if an inline width attribute is used as a default when sizes is omitted (in the case of extending this logic to srcset calculations), then it may be preferential for the consumer to only use the width value when it's smaller than the viewport width.

@igrigorik

This comment has been minimized.

Show comment
Hide comment
@igrigorik

igrigorik Sep 15, 2015

bump ... any blockers to resolve this? This is an important feature for Client Hints.

igrigorik commented Sep 15, 2015

bump ... any blockers to resolve this? This is an important feature for Client Hints.

@tabatkins

This comment has been minimized.

Show comment
Hide comment
@tabatkins

tabatkins Sep 16, 2015

I'm down with this. It makes sense to me.

tabatkins commented Sep 16, 2015

I'm down with this. It makes sense to me.

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Sep 16, 2015

OK with me, but we should ignore it when it uses percentages (since it refers to containing block, and it's invalid anyway). Also I'm not sure I like @joemcgill's idea to have the viewport width be an upper bound, that seems unnecessary magic that could get confusing results, especially for Client Hints. You can still achieve the effect by using the sizes attribute, but then it's explicit and intentional.

zcorpan commented Sep 16, 2015

OK with me, but we should ignore it when it uses percentages (since it refers to containing block, and it's invalid anyway). Also I'm not sure I like @joemcgill's idea to have the viewport width be an upper bound, that seems unnecessary magic that could get confusing results, especially for Client Hints. You can still achieve the effect by using the sizes attribute, but then it's explicit and intentional.

@joemcgill

This comment has been minimized.

Show comment
Hide comment
@joemcgill

joemcgill Sep 16, 2015

seems unnecessary magic that could get confusing results

I agree. Probably best to keep things straightforward.

joemcgill commented Sep 16, 2015

seems unnecessary magic that could get confusing results

I agree. Probably best to keep things straightforward.

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