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: alt text no longer cause images to render at wrong dimensions #146

Merged
merged 7 commits into from
Jun 29, 2018

Conversation

frederickfogerty
Copy link
Contributor

@frederickfogerty frederickfogerty commented Jun 26, 2018

Description

Previously, if a user wished to add alt text to an image it would cause the image to be displayed at weird dimensions depending on which browser the user was using. This was due to two reasons: a) in some browsers, including alt text meant that empty images would render at dimensions other than 0x0, and b) in some browsers, that the first render pass of this component (which renders an empty image) would cause an empty, invalid, image to render at dimensions other than 0x0.

Rendering at 0x0 is important for this library to work out what the dimensions of the requested image should be. If the empty image renders at 0x0, this means that the user has not constrained the size of the image, and that a full-res image should be requested. If the empty image renders at anything other than 0x0, this means that the user has constrained the image in some way, and that we should only request an image for those dimensions.

This PR fixes these problems in two ways. First, it only adds the alt text after the first render pass (or if in aggressiveLoad mode). Before the second render pass, alt is empty. Second, the src for an empty image is a data-uri of a 1x1 transparent gif. Before now, the component rendered an invalid image (as it had no src), which caused browser behaviour to be undefined. Now, with the new src, the image is valid, and browsers render the image consistently.

This fixes #41.

Bug Fix

  • All existing unit tests are still passing (if applicable)
  • Add new passing unit tests to cover the code introduced by your PR
  • Update the readme
  • Update or add any necessary API documentation
  • Add some steps so we can test your bug fix
  • The PR title is in the conventional commit format: e.g. fix(<area>): fixed bug #issue-number
  • Add your info to the contributors array in package.json!

Steps to Test

Review the tests in test/integration/react-imgix.test.js and test/unit/react-imgix.test.js

@frederickfogerty frederickfogerty merged commit d3183a6 into master Jun 29, 2018
@frederickfogerty frederickfogerty deleted the fred/fix-41 branch June 29, 2018 23:24
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.

Adding an alt prop to imgProps loads a low res image
2 participants