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

Automatically set srcSet attributes #4

Merged
merged 1 commit into from
Nov 17, 2015
Merged

Conversation

ekosz
Copy link
Contributor

@ekosz ekosz commented Nov 17, 2015

This adds a new option called srcSet which defaults to true. When enabled, the 2x and 3x srcSet attributes will be added to rendered component. Currently, the attribute will only be added if the rendered component is an img.

What do you all think? I don't know if the option name is the best at the moment.

@kellysutton
Copy link
Contributor

Two cents on the naming: the flag should likely be named includeSrcset or generateSrcset as to not be confused with an actual srcset value. What do you think?

@ekosz
Copy link
Contributor Author

ekosz commented Nov 17, 2015

Yep! I was originally going to do that, but it didn't seem to matchup with the other options naming-wise. I can make the change now. Also do you think this option should be enabled by default?

@kellysutton
Copy link
Contributor

Because srcset is a valid <img> attribute, I think it will need a prefix like include or generate.

Because srcset merely upgrades the functionality of an image element, there's no harm in including it even when it is not supported. Thus, I think it should be enabled by default.

This adds a new option called `generateSrcSet` which defaults to true. When
enabled, the 2x and 3x srcSet attributes will be added to rendered component.
Currently, the attribute will only be added if the rendered component is an
`img`.
@ekosz
Copy link
Contributor Author

ekosz commented Nov 17, 2015

@kellysutton Alright, I've updated the PR with the changes.

kellysutton pushed a commit that referenced this pull request Nov 17, 2015
Automatically set srcSet attributes
@kellysutton kellysutton merged commit 01c72ce into imgix:master Nov 17, 2015
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.

2 participants