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] Get the asset scale which is closest to the window scale #1408

Closed

Conversation

EvanBacon
Copy link
Contributor

const preferredScale = window.devicePixelRatio;
// Get the scale which is closest to the preferred scale
scale = asset.scales.reduce((prev, curr) =>
Math.abs(curr - preferredScale) < Math.abs(prev - preferredScale) ? curr : prev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to round the size so you always get the most crisp image.

@necolas
Copy link
Owner

necolas commented Sep 30, 2019

Please could you add unit tests for this feature?

@ericvicenti
Copy link

  1. This fix looks great for most apps
  2. It will totally screw up server-rendering, because window.pixelRatio is only on the client, and images will loaded twice at different resolutions

So I agree this is the simplest and preferred option for normal client apps. But for SSR+client apps, maybe we could consider an implementation with <img srcset=..?

@necolas
Copy link
Owner

necolas commented Oct 8, 2019

This is why it needs to use PixelRatio, where the values can be set on the server based on client hints. source is also not rendered on the server, so this would only be for defaultSource.

@ericvicenti
Copy link

Relying on client hints is quite imprecise, and it breaks/complicates caching.

Even if you could predict the window size from the server, the required resolution depends on the layout in the app. Maybe a larger image is justified on mobile if it takes up the full screen, in contrast to an icon.

@necolas, you've been much deeper into web eng than me for the past few years. Can I ask why you're not interested in using srcset? For these reasons I think it is more appropriate for the browser to determine which resolution to download.

Here's a nice article on it.. https://medium.com/hceverything/applying-srcset-choosing-the-right-sizes-for-responsive-images-at-different-breakpoints-a0433450a4a3

@MoOx
Copy link
Contributor

MoOx commented Dec 9, 2019

I also think we should keep cached SSR in mind (static rendering) by using srcset somewhere... I know it will be harder since we are currently using a background image with a img overlay for accessibility, but maybe we could go for a simpler img + srcset + object-fit, don't you think?

@msand
Copy link

msand commented Jan 2, 2020

Would it be possible to expose the resolveAssetUri helper? Or some way to get the uri from a require('./img.ext') call? It's needed for implementing svg raster image embedding in the web support of react-native-svg

Current ugly workaround / hack: (There must be a better way)

https://snack.expo.io/@msand/react-native-svg-example
used in ./Examples/Image.js
defined in ./components/SnackImageCompat.web.js

import React, { Component, Fragment } from 'react';
import { Image as NativeImage, View } from 'react-native';
import { Image as SvgImage } from 'react-native-svg';

class WebImage extends NativeImage {
  constructor(props, context) {
    super(props, context);
    this._setImageRef = ref => {
      this._imageRef = ref;
      const attrs = ref && ref.attributes;
      const src = attrs && attrs.src;
      const uri = src && src.value;
      this.setState({ href: uri });
    };
    this.oldRender = this.render;
    this.render = () => {
      const uri = this.state && this.state.href;
      return (
        <Fragment>
          <View
            style={{
              visibility: 'hidden',
              position: 'absolute',
              width: 0,
              height: 0,
            }}>
            {this.oldRender()}
          </View>
          <SvgImage {...this.props} href={uri} />
        </Fragment>
      );
    };
  }
}

export default props => (
  <WebImage {...props} source={props.source || props.href} />
);

@EvanBacon
Copy link
Contributor Author

Closing in favor of #1456

@EvanBacon EvanBacon closed this Jan 3, 2020
@msand
Copy link

msand commented Jan 3, 2020

@EvanBacon Is there any other way to get the uri of images in snack / expo web?

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.

5 participants