aspectRatio should be optional?

[smeijer]

Perhaps I'm missing the point of aspectRatio, but I cannot see why this field is required. Even better; I don't see why this should be a api property at all.

Isn't the aspectRatio simply width / height? Anything else results in distortion no?

Photos properties

Property	Type	Default	Description
width	number	undefined	Required. Width of the gallery image
height	number	undefined	Required. Height of the gallery image
aspectRatio	number	undefined	Required. Aspect ratio of the gallery image (width / height)

[mwitczak]

You have the point. One of the three properties could be calculated from the two others. If I didn't miss anything, looking at the code, you can see that width property is ignored at all. It is directly calculated from height * aspectRatio. It would make sense to remove either width or aspectRatio from Props.

[smeijer]

I started fixing this one myself; but ended up in a rewrite of this library. So I didn't feel for a PR anymore. To give something back; I just posted it as a gist. You can take a look; in case you might need some inspiration / an other point of view.

https://gist.github.com/smeijer/02638a47c5dd31e64ddf84c2cb62cc12

This one also solves #28. But I did remove the responsiveness. Because I believe that it is something that shouldn't be in this lib. I solve by wrapping the component in react-measure and adjusting the cols property.

[neptunian]

@smeijer does your approach to addressing the responsiveness aspect simply add columns when the container gets wide or does it resize and grow the images like what this library currently does? Thanks for the insights.

[smeijer]

@neptunian, my approach is a combination, but out of the scope of that gist. The gallery it self, scales the images to fit the width of the container. Like this library does. Trough react-measure I measure the width of the container, and adjust the number of columns.

For example; if react-measure says the width is 1300 pixels, and I want a maximum image size of 250 pixels, a simple Math.ceil(1300 / 250) will tell Gallery to render 6 columns. The gallery will scale the images to fit, so they will be approx 216 pixels wide (ignoring margin and different image ratios here for sake of simplicity).

I can add the gallery like:

// written in the comment to show the idea, so bugs are likely to exists

render() {
  const { maxImageWidth } = this.props;

  return (
    <Measure whitelist={['width']}>{({ width }) => (
      <Gallery cols={Math.ceil(width / maxImageWidth)}>....</Gallery>
     )}
    </Measure>
  );
}

Maximum or Minimum width can easiliy be changed by changing Math.ceil with Math.floor.

[neptunian]

@smeijer I see. Okay i'll think about using this measure tool for adding more columns based on the container width. thanks!

[neptunian]

@smeijer I tried your solution with Measure component but it gives me the width of the app and not the Gallery div's width. Is there any way to measure the actual Gallery component so as to pass in the appropriate number of columns based on the Gallery's width and not the app's width?

[smeijer]

In my opinion the Gallery should take the full width of its parent. It's not up to the Gallery to decide how wide it should become. If the user wants to limit it's width, he can simply do so with a parent, or if you want with a pass-trough style property.

<div style={{ width: 650 }}>
  <Gallery photos={[...]} />
</div>

<Gallery photos={[...]} style={{ width: 600 }} />

Gallery should here have a render method like:

render() {
  const { photos, style } = this.props;

  return (
    <div style={{ width: '100%', ...style }}>
      {photos.map(...)}
    </div>
  )
}

React Measure will return the width of the element he is surrounding. This is here either the 100% of the parent, or the by the user limited number or pixels trough a style property.