react-native icon indicating copy to clipboard operation
react-native copied to clipboard

feat: added `crossOrigin`, `referrerPolicy`, `srcSet` and `src` props to the Image Component.

Open dhruvtailor7 opened this issue 1 year ago • 8 comments

Summary

This PR is for adding the support for crossOrigin, referrerPolicy and srcSet props to Image Component and mapping the src prop to source.uri of Image Component for the issue https://github.com/facebook/react-native/issues/34424. Example is also added in the RNTester ImageExample.

Changelog

[General] [Changed] - Map the src prop to source.uri prop in Image Component. [General] [Added] - added crossOrigin, referrerPolicy and srcSet props to Image Component.

Test Plan

  1. Navigate to Image Component Example in the RNTester app.
  2. Contains an example of the Image component using the src prop.

dhruvtailor7 avatar Aug 23 '22 12:08 dhruvtailor7

Hi @dhruvtailor7!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Aug 23 '22 12:08 facebook-github-bot

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,637,870 +835
android hermes armeabi-v7a 7,050,076 +842
android hermes x86 7,939,664 +826
android hermes x86_64 7,911,593 +822
android jsc arm64-v8a 9,513,421 +592
android jsc armeabi-v7a 8,288,943 +609
android jsc x86 9,452,810 +595
android jsc x86_64 10,043,953 +590

Base commit: 4f6929bb3c4b4d8ba53fa250645b5ae4e297a1b3 Branch: main

analysis-bot avatar Aug 23 '22 12:08 analysis-bot

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot avatar Aug 23 '22 19:08 facebook-github-bot

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 4f6929bb3c4b4d8ba53fa250645b5ae4e297a1b3 Branch: main

analysis-bot avatar Aug 24 '22 08:08 analysis-bot

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 24 '22 16:08 facebook-github-bot

Can we tackle all the new Image props at the same time, given that they all combine into the existing source prop?

e.g.,

const { crossOrigin, height, referrerPolicy, src, srcSet, width } = props;
let source = null;
if (src != null) {
  const headers = {};
  if (crossOrigin === 'use-credentials') {
    headers['Access-Control-Allow-Credentials'] = true;
  }
  if (referrerPolicy != null) {
    headers['Referrer-Policy'] = referrerPolicy;
  }
  nextProps.progressiveRenderingEnabled = true;
  source = { headers, height, uri: src, width };
}
if (srcSet != null) {
  source = [];
  const srcList = srcSet.split(', ');
  srcList.forEach((src) => {
    const [uri, xscale] = src.split(' ');
    const scale = parseInt(xscale.split('x')[0], 10);
    const headers = {};
    if (crossOrigin === 'use-credentials') {
      headers['Access-Control-Allow-Credentials'] = true;
    }
    if (referrerPolicy != null) {
      headers['Referrer-Policy'] = referrerPolicy;
    }
    source.push({ headers, height, scale, uri, width });
  });
}

necolas avatar Aug 24 '22 18:08 necolas

@necolas on Android, if we have multiple items in the source prop, and we do not provide width/height on each item, than an error is thrown from the native side. So, if we map multiple sources using the srcSet prop then the same error will be thrown as there is no way to provide width/height to each item using srcSet.

dhruvtailor7 avatar Aug 27 '22 15:08 dhruvtailor7

The same width and height should be passed for each source

necolas avatar Aug 27 '22 15:08 necolas

@dhruvtailor7 are you planning to update this PR? Thanks

necolas avatar Aug 31 '22 15:08 necolas

@dhruvtailor7 are you planning to update this PR? Thanks

@necolas Yes, I am still looking into the failing flow test cases.

dhruvtailor7 avatar Sep 01 '22 14:09 dhruvtailor7

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 05 '22 09:09 facebook-github-bot

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 07 '22 12:09 facebook-github-bot

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 07 '22 18:09 facebook-github-bot

This pull request was successfully merged by @dhruvtailor7 in 47a05bc26ab76add640183c1d9d8cbba39d7d0d2.

When will my fix make it into a release? | Upcoming Releases

react-native-bot avatar Sep 07 '22 20:09 react-native-bot