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

Srcsets - better handling of multiple srcsets #744

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

fallax
Copy link
Contributor

@fallax fallax commented Jul 29, 2022

The previous fix to #724 didn't handle this situation:

srcset="test.webp 1.5x, test2.webp 2x"

This PR should fix this, add a new test case, and hopefully remove some code duplication as a nice bonus.

@fallax fallax requested a review from gjtorikian as a code owner July 29, 2022 13:46
@gjtorikian
Copy link
Owner

Oh, thanks very much! Releasing this as a patch update right now.

@gjtorikian gjtorikian merged commit e48f609 into gjtorikian:main Jul 29, 2022
@fallax
Copy link
Contributor Author

fallax commented Jul 29, 2022

Looks like this still leaves a few issues. 'img' tags with multiple pixel density srcset attributes now work correctly - but 'source' tags don't.

I think this can be fixed by switching 'source' tags to being handled by the code in images.rb rather than the code in links.rb. I'm happy to contribute a PR to do this. Does that sound like a sensible approach to you? If so I'll get a second PR going...

@gjtorikian
Copy link
Owner

Sure, I can review the change and see what makes most sense then.

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