Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

[RelatedArtists] Use layoutEvent instead of Dimensions #400

Merged
merged 2 commits into from
Nov 30, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 23 additions & 26 deletions lib/components/related_artists/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

import Relay from 'react-relay'
import React from 'react'
import { StyleSheet, View, Dimensions } from 'react-native'
import { StyleSheet, View } from 'react-native'

import SerifText from '../text/serif'
import RelatedArtist from './related_artist'
import type { LayoutEvent } from '../../system/events'

class RelatedArtists extends React.Component {
state: {
Expand All @@ -16,31 +17,27 @@ class RelatedArtists extends React.Component {

constructor(props) {
super(props)
this.state = this.layoutState()
this.state = {
columns: 0,
imageSize: {
width: 1,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

opaque_image_view needs some sort of aspect ratio or dimensions to render, so I start with 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to hear suggestions though because this does feel a bit inelegant. I'm guessing this could be fixed by doing some work in the image view or by not rendering images at all until after layout?

Copy link
Contributor

Choose a reason for hiding this comment

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

@alloy ^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can stay as it is for now because the image view will throw an error if you try otherwise.

Copy link
Contributor

@alloy alloy Dec 1, 2016

Choose a reason for hiding this comment

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

Yeah the image view wants one or the other so that it can infer if you want to resize or crop. I would say that the simplest solution here is to not render the image view yet until after the onLayout callback is performed and e.g. columnCount > 0.

height: 1,
},
}


}

layoutState() : Object {
const width = Dimensions.get('window').width
const isPad = width > 700
const isPadHorizontal = width > 1000

// TODO: Document what these margins are based on.
let columnCount
let margins
if (isPad) {
if (isPadHorizontal) {
columnCount = 4
margins = 140
} else {
columnCount = 3
margins = 100
}
} else {
columnCount = 2
margins = 60
}
layoutState(currentLayout) : Object {
const width = currentLayout.width
const isPad = width > 600
const isPadHorizontal = width > 900

const columnCount = isPad ? (isPadHorizontal ? 4 : 3) : 2
const marginWidth = 20
const totalMargins = marginWidth * (columnCount - 1)

const imageWidth = (width - margins) / columnCount
const imageWidth = (width - totalMargins) / columnCount
const imageHeight = imageWidth / 1.5

return {
Expand All @@ -52,16 +49,16 @@ class RelatedArtists extends React.Component {
}
}

onLayout = (e) => {
const newLayoutState = this.layoutState()
onLayout = (event: LayoutEvent) => {
const newLayoutState = this.layoutState(event.nativeEvent.layout)
if (this.state.columns !== newLayoutState.columns) {
this.setState(newLayoutState)
}
}

render() {
return (
<View style={styles.container} onLayout={this.onLayout}>
<View style={styles.container} onLayout={this.onLayout.bind(this)}>
<SerifText style={styles.heading}>Related Artists</SerifText>
<View style={styles.artistContainer}>
{ this.renderArtists() }
Expand Down