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

Image: modern API and implementation #1786

Open
necolas opened this issue Oct 27, 2020 · 8 comments
Open

Image: modern API and implementation #1786

necolas opened this issue Oct 27, 2020 · 8 comments
Labels
project:react-native-web Issue associated with react-native-web
Milestone

Comments

@necolas
Copy link
Owner

necolas commented Oct 27, 2020

Is your feature request related to a problem? Please describe.

Provider a high-performance Image implementation for web. This issue is to discuss how we should rewrite Image to support modern web features.

Describe a solution you'd like

The new Image should implement the following feature requests…

  1. Support browser ML-generated alt text.
  2. Support browser lazy loading.
  3. Support browser crossOrigin configuration.
  4. Display image in Windows High Contrast mode.
  5. Support image DPI switching.
  6. Support for CSS like backgroundBlendMode.
  7. Available for SSR.

The new Image will not support the following APIs…

  1. No tintColor, resizeMode, and shadow* styles.
  2. No repeat value for resizeMode.

The proposed Image API looks like this…

<Image
  alternativeText=''
  crossOrigin='anonymous'
  decoding='async'
  draggable='false'
  focusable='false'
  loading='lazy'
  nativeID=''
  onError={() => {}}
  onLoad={() => {}}
  source={[ {uri, scale}, {uri, scale} ]}
  style={{}}
  testID=''
/>

Any dynamic loading via XHR/fetch would have to be layered upon that.

Additional context

Please comment with addition context and use cases.

@comp615
Copy link
Contributor

comp615 commented Oct 28, 2020

We have two uses for the image queryCache I wanted to share here

  1. When dataSaver is on, we try and get the largest cached variant already loaded (and use that instead) to save data.
// If data saver is on, then attempt to use the highest quality image we've already loaded to save data,
  // Exclude tiny variant (pre-data) saver images, so that we have a minimum quality
  const dataSaverCachedSource =
    dataSaver &&
    loadingVariant &&
    Math.max(loadingVariant.width, loadingVariant.height) >= MIN_RESOLUTION_CACHED_DATA_SAVER_THRESHOLD
      ? loadingVariant.uri
      : null;
  const src = dataSaverCachedSource || _selectSource(photo);
  1. We use defaultSource to show the largest loaded image while the full-size variant loads:
<Image
        defaultSource={loadingVariant && loadingVariant.uri}
        draggable
        onContextMenu={onContextMenu}
        // If there's a fallback image showing, that's good enough and we can swallow the error
        onError={loadingVariant ? undefined : onError}
        onLoad={onLoad}
       // These just set internal state and shows a little loading spinner so people know we are upscaling the image
        onLoadEnd={this._handleLoadEnd}
        onLoadStart={this._handleLoadStart}
        source={src}
      />

@RichardLindhout
Copy link
Contributor

RichardLindhout commented Nov 21, 2020

Looking forward to this change!

Some possible solutions for implementing some of the props with CSS:

It would be cool if React Native could add something similar to object-position otherwise it would be nice if we could use it in React Native Web somehow.

@necolas necolas modified the milestones: 0.15, 0.16 Dec 8, 2020
@viggyfresh
Copy link

@necolas just to make sure I'm understanding correctly - the proposed new API will be in tension with the React Native API such that it won't have built-in support for HTTP verbs like headers, for example, in the source property?

as this is already not present in the react-native-web Image implementation, this would be fine by our team 😄 i was hopeful that #1470 (or what looks to be a rebased version in #1843) would land, but I'm of course mindful of the fact that the React Native Image component API isn't aligned with the direction you'd like to go!

our use case, FWIW, is secure images served behind an authentication layer in our API server; i'm not sure how common of a use case this is. if there is a way to somehow shoehorn this in, I would love to see that or help make that happen. If not, our web-specific workarounds will live to see another day 😸

@jamesisaac
Copy link

I'd give a +1 to @viggyfresh's use-case of wanting to serve images behind auth.

For example, Expo (which wraps RNWeb) doesn't really have cookie support, so instead suggests using token-based auth: expo/expo#6756 (comment) . (Even if cookies were supported, I believe having all client-side data stored together in LocalStorage, and communicated via http headers, is a more modern approach).

So without support for cookies or http headers when serving images, I think the only option remaining becomes to stick the auth token in the querystring. I suppose there's not really anything wrong with that, but for consistency it would be nice if the same auth mechanism could be used everywhere.

@petrbela
Copy link
Contributor

petrbela commented Mar 14, 2021

@jamesisaac

I think the only option remaining becomes to stick the auth token in the querystring. I suppose there's not really anything wrong with that

Sending private tokens in the query string is not recommended because unlike headers, they often show up in logs.

@comp615
Copy link
Contributor

comp615 commented Aug 25, 2021

Adding one more idea here for the image rework. Right now I think it's impossible to load images with crossorigin="anonymous". In my testing, even if I go into RNW source and add it to the loader and hidden image, there's no equivalent attribute for background image so two requests get made, one properly anon from the ImageLoader, but then a second from background-image.
image

@janicduplessis
Copy link
Contributor

janicduplessis commented Aug 27, 2021

I've built this as an alternative to the RN web implementation if that can provide ideas for improvements. Def has lots of tradeoffs and doesn't implement all RN features.

The main idea is to extend the source prop with a sources config that pretty much matches https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source. This sources data can be generated by a webpack plugin for local images or returned by a backend for remote images. This allow supporting different sizes for densities via srcSet or different image formats.

https://github.com/th3rdwave/web-image

@comp615
Copy link
Contributor

comp615 commented Jan 27, 2022

#2171 is covered by this as well

@necolas necolas added the project:react-native-web Issue associated with react-native-web label Jul 2, 2022
Repository owner deleted a comment from blackbing Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project:react-native-web Issue associated with react-native-web
Projects
None yet
Development

No branches or pull requests

7 participants