Skip to content

Add the option to pass headers to useImageDimensions #225

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

Merged
merged 6 commits into from
Oct 18, 2021

Conversation

thibmaek
Copy link
Contributor

@thibmaek thibmaek commented Mar 25, 2021

Summary

When using headers (for example for authentication) together with Image, you could not use useImageDimensions because it only included Image.getSize to determine the size of a remote image. This PR adds support for passing a second optional headers param to useImageDimensions which will then use Image.getSizeWithHeaders from React Native.

Test Plan

const ImageViewer = ({ uri, headers }) => {
  const { width, height } = useImageDimensions(uri, headers)
  return <Image style={{ width, height }} />
}

<ImageViewer uri="https://...." headers={{ authorization: `Bearer  ....` }} />

What's required for testing (prerequisites)?

A remote resource which requires a header (e.g auth header)

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android
Web

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)
  • I've created a snack to demonstrate the changes: LINK HERE

Notes

I edited this PR via Github, let me know if it requires changes

@GertjanReynaert
Copy link
Contributor

Hey @LinusU, sorry for the direct mention, but I'm trying to get some movement in this PR.

I've fixed the CI issues for this PR and I'm wondering if you'd be able to take a look at it to see if it's good to merge/release.

@LinusU
Copy link
Member

LinusU commented Oct 18, 2021

Sorry for not looking at this earlier, happy to merge if you can rebase on latest master ☺️

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👍

@LinusU LinusU added minor Increment the minor version when merged release Create a release when this pr is merged labels Oct 18, 2021
@LinusU LinusU merged commit 6cf86b5 into react-native-community:master Oct 18, 2021
@pvinis
Copy link
Member

pvinis commented Oct 18, 2021

🚀 PR was released in v2.7.0 🚀

@pvinis pvinis added the released This issue/pull request has been released. label Oct 18, 2021
@GertjanReynaert GertjanReynaert deleted the patch-1 branch October 18, 2021 15:34
@github-actions github-actions bot mentioned this pull request Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants