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

Fix srcset support #93

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Cardinal90
Copy link

Justification is in commit comments

@alexanderdickson
Copy link
Owner

Hi @Cardinal90! Thanks for the PR.

Some things

  • You test against undefined, I guess because they can return empty strings?
  • Can you add relevant tests? There is a qunit file.
  • Can you bump the version to 2.5? It should be automatically published to npm now with a recent change

Thanks!

@Cardinal90
Copy link
Author

Hi, sorry for the long delay.

  • That check is done against new Image() with no attributes. If srcset is supported, it is an empty string, else undefined.
  • I don't see a good way to add tests in this case. You'd have to assert that srcset and sizes attributes are properly copied onto an internal image instance. From the perspective of library's user (or the test file) the callback is called either way. With src attribute of about: all images in your tests error out anyway, but even using real images I don't see how to test this.
  • Bumped version.

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