Skip to content

Commit d4c63e8

Browse files
hawkriveshawkrives
authored
Merge pull request #1546 from StoDevX/building-hours-list-fix
Optimize Building Hours views (and make them actually update)
2 parents b9cd90f + 34de310 commit d4c63e8

File tree

5 files changed

+146
-82
lines changed

5 files changed

+146
-82
lines changed

‎source/views/building-hours/detail/building.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,20 @@ const styles = StyleSheet.create({
3333
},
3434
})
3535

36-
export class BuildingDetail extends React.PureComponent {
37-
props: {info: BuildingType, now: moment, onProblemReport: () => any}
36+
type Props = {
37+
info: BuildingType,
38+
now: moment,
39+
onProblemReport: () => any,
40+
}
41+
42+
export class BuildingDetail extends React.Component<void, Props, void> {
43+
shouldComponentUpdate(nextProps: Props) {
44+
return (
45+
!this.props.now.isSame(nextProps.now, 'minute') ||
46+
this.props.info !== nextProps.info ||
47+
this.props.onProblemReport !== nextProps.onProblemReport
48+
)
49+
}
3850

3951
render() {
4052
const {info, now, onProblemReport} = this.props

‎source/views/building-hours/detail/index.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ export class BuildingHoursDetailView extends React.PureComponent {
3232
componentWillMount() {
3333
// This updates the screen every ten seconds, so that the building
3434
// info statuses are updated without needing to leave and come back.
35-
this.setState({intervalId: setInterval(this.updateTime, 10000)})
35+
this.setState({intervalId: setInterval(this.updateTime, 1000)})
3636
}
3737

3838
componentWillUnmount() {
3939
clearTimeout(this.state.intervalId)
4040
}
4141

4242
updateTime = () => {
43-
this.setState({now: moment.tz(CENTRAL_TZ)})
43+
this.setState(() => ({now: moment.tz(CENTRAL_TZ)}))
4444
}
4545

4646
reportProblem = () => {

‎source/views/building-hours/list.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export class BuildingHoursList extends React.PureComponent {
5555
/>
5656
}
5757
sections={(this.props.buildings: any)}
58+
extraData={this.props}
5859
keyExtractor={this.keyExtractor}
5960
renderSectionHeader={this.renderSectionHeader}
6061
renderItem={this.renderItem}

‎source/views/building-hours/row.js‎

Lines changed: 112 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@
66
import React from 'react'
77
import {View, Text, StyleSheet} from 'react-native'
88
import {Badge} from '../components/badge'
9+
import isEqual from 'lodash/isEqual'
910
import type momentT from 'moment'
1011
import type {BuildingType} from './types'
1112
import * as c from '../components/colors'
12-
import {Row, Column} from '../components/layout'
13+
import {Row} from '../components/layout'
1314
import {ListRow, Detail, Title} from '../components/list'
1415
import {getDetailedBuildingStatus, getShortBuildingStatus} from './lib'
1516

@@ -42,68 +43,128 @@ const styles = StyleSheet.create({
4243
},
4344
})
4445

45-
export class BuildingRow extends React.PureComponent {
46-
props: {
47-
info: BuildingType,
48-
name: string,
49-
now: momentT,
50-
onPress: BuildingType => any,
46+
const BG_COLORS = {
47+
Open: c.moneyGreen,
48+
Closed: c.salmon,
49+
}
50+
51+
const FG_COLORS = {
52+
Open: c.hollyGreen,
53+
Closed: c.brickRed,
54+
}
55+
56+
type Props = {
57+
info: BuildingType,
58+
name: string,
59+
now: momentT,
60+
onPress: BuildingType => any,
61+
}
62+
63+
type State = {
64+
openStatus: string,
65+
hours: Array<any>,
66+
accentBg: string,
67+
accentText: string,
68+
}
69+
70+
export class BuildingRow extends React.Component<void, Props, State> {
71+
state = {
72+
openStatus: 'Unknown',
73+
hours: [],
74+
firstUpdate: true,
75+
76+
// when changing these, make sure to change the fallbacks in setStateFromProps
77+
accentBg: c.goldenrod,
78+
accentText: 'rgb(130, 82, 45)',
5179
}
5280

53-
onPress = () => {
54-
this.props.onPress(this.props.info)
81+
componentWillMount() {
82+
this.setStateFromProps(this.props)
5583
}
5684

57-
render() {
58-
const {info, name, now} = this.props
85+
componentWillReceiveProps(nextProps: Props) {
86+
this.setStateFromProps(nextProps)
87+
}
5988

60-
const bgColors = {
61-
Open: c.moneyGreen,
62-
Closed: c.salmon,
63-
}
64-
const foregroundColors = {
65-
Open: c.hollyGreen,
66-
Closed: c.brickRed,
89+
shouldComponentUpdate(nextProps: Props, nextState: State) {
90+
// We won't check the time in shouldComponentUpdate, because we really
91+
// only care if the building status has changed, and this is called after
92+
// setStateFromProps runs.
93+
return (
94+
this.props.name !== nextProps.name ||
95+
this.props.info !== nextProps.info ||
96+
this.props.onPress !== nextProps.onPress ||
97+
this.state.openStatus !== nextState.openStatus ||
98+
!isEqual(this.state.hours, nextState.hours)
99+
)
100+
}
101+
102+
setStateFromProps = (nextProps: Props) => {
103+
// we check the time in setStateFromProps, because shouldComponentUpdate
104+
// runs _after_ setStateFromProps.
105+
if (
106+
this.props.now.isSame(nextProps.now, 'minute') &&
107+
!this.state.firstUpdate
108+
) {
109+
return
67110
}
68111

112+
const {info, now} = nextProps
113+
69114
const openStatus = getShortBuildingStatus(info, now)
70115
const hours = getDetailedBuildingStatus(info, now)
71116

72-
const accent = bgColors[openStatus] || c.goldenrod
73-
const textaccent = foregroundColors[openStatus] || 'rgb(130, 82, 45)'
117+
// when changing these, make sure to change the initial values in `state`
118+
const accentBg = BG_COLORS[openStatus] || c.goldenrod
119+
const accentText = FG_COLORS[openStatus] || 'rgb(130, 82, 45)'
120+
121+
this.setState(() => ({
122+
openStatus,
123+
hours,
124+
accentBg,
125+
accentText,
126+
firstUpdate: false,
127+
}))
128+
}
129+
130+
onPress = () => {
131+
this.props.onPress(this.props.info)
132+
}
133+
134+
render() {
135+
const {info, name} = this.props
136+
const {openStatus, hours, accentBg, accentText} = this.state
74137

75138
return (
76139
<ListRow onPress={this.onPress} arrowPosition="center" style={styles.row}>
77-
<Column>
78-
<Row style={styles.title}>
79-
<Title lines={1} style={styles.titleText}>
80-
<Text>{name}</Text>
81-
{info.abbreviation ? <Text> ({info.abbreviation})</Text> : null}
82-
{info.subtitle
83-
? <Text style={styles.subtitleText}> {info.subtitle}</Text>
84-
: null}
85-
</Title>
86-
87-
<Badge
88-
text={openStatus}
89-
accentColor={accent}
90-
textColor={textaccent}
91-
style={styles.accessoryBadge}
92-
/>
93-
</Row>
94-
95-
<View style={styles.detailWrapper}>
96-
{hours.map(({isActive, label, status}, i) =>
97-
<Detail key={i} style={styles.detailRow}>
98-
<BuildingTimeSlot
99-
highlight={hours.length > 1 && isActive}
100-
label={label}
101-
status={status}
102-
/>
103-
</Detail>,
104-
)}
105-
</View>
106-
</Column>
140+
<Row style={styles.title}>
141+
<Title lines={1} style={styles.titleText}>
142+
<Text>{name}</Text>
143+
{info.abbreviation ? <Text> ({info.abbreviation})</Text> : null}
144+
{info.subtitle
145+
? <Text style={styles.subtitleText}> {info.subtitle}</Text>
146+
: null}
147+
</Title>
148+
149+
<Badge
150+
text={openStatus}
151+
accentColor={accentBg}
152+
textColor={accentText}
153+
style={styles.accessoryBadge}
154+
/>
155+
</Row>
156+
157+
<View style={styles.detailWrapper}>
158+
{hours.map(({isActive, label, status}, i) =>
159+
<Detail key={i} style={styles.detailRow}>
160+
<BuildingTimeSlot
161+
highlight={hours.length > 1 && isActive}
162+
label={label}
163+
status={status}
164+
/>
165+
</Detail>,
166+
)}
167+
</View>
107168
</ListRow>
108169
)
109170
}
@@ -119,7 +180,7 @@ const BuildingTimeSlot = ({
119180
highlight: boolean,
120181
}) => {
121182
// we don't want to show the 'Hours' label, since almost every row has it
122-
const showLabel = label !== 'Hours'
183+
const showLabel = label && label !== 'Hours'
123184

124185
return (
125186
<Text>

‎source/views/building-hours/stateful-list.js‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@
77

88
import React from 'react'
99
import {NoticeView} from '../components/notice'
10-
import {tracker} from '../../analytics'
11-
import bugsnag from '../../bugsnag'
1210
import {BuildingHoursList} from './list'
1311

1412
import moment from 'moment-timezone'
1513
import type {TopLevelViewPropsType} from '../types'
1614
import type {BuildingType} from './types'
17-
import {data as fallbackBuildingHours} from '../../../docs/building-hours'
15+
import * as defaultData from '../../../docs/building-hours'
16+
import {reportNetworkProblem} from '../../lib/report-network-problem'
1817
import toPairs from 'lodash/toPairs'
1918
import groupBy from 'lodash/groupBy'
2019

2120
import {CENTRAL_TZ} from './lib'
22-
const githubBaseUrl = 'https://stodevx.github.io/AAO-React-Native'
21+
const githubBaseUrl =
22+
'https://stodevx.github.io/AAO-React-Native/building-hours.json'
2323

2424
const groupBuildings = (buildings: BuildingType[]) => {
2525
const grouped = groupBy(buildings, b => b.category || 'Other')
@@ -42,10 +42,10 @@ export class BuildingHoursView extends React.Component {
4242
intervalId: number,
4343
} = {
4444
error: null,
45-
loading: true,
45+
loading: false,
4646
// now: moment.tz('Wed 7:25pm', 'ddd h:mma', null, CENTRAL_TZ),
4747
now: moment.tz(CENTRAL_TZ),
48-
buildings: groupBuildings(fallbackBuildingHours),
48+
buildings: groupBuildings(defaultData.data),
4949
intervalId: 0,
5050
}
5151

@@ -54,46 +54,36 @@ export class BuildingHoursView extends React.Component {
5454

5555
// This updates the screen every ten seconds, so that the building
5656
// info statuses are updated without needing to leave and come back.
57-
this.setState({intervalId: setInterval(this.updateTime, 10000)})
57+
this.setState({intervalId: setInterval(this.updateTime, 1000)})
5858
}
5959

6060
componentWillUnmount() {
6161
clearTimeout(this.state.intervalId)
6262
}
6363

6464
updateTime = () => {
65-
this.setState({now: moment.tz(CENTRAL_TZ)})
65+
this.setState(() => ({now: moment.tz(CENTRAL_TZ)}))
6666
}
6767

6868
fetchData = async () => {
69-
this.setState({loading: true})
70-
71-
let buildings: BuildingType[] = []
72-
try {
73-
let container = await fetchJson(`${githubBaseUrl}/building-hours.json`)
74-
let data = container.data
75-
buildings = data
76-
} catch (err) {
77-
tracker.trackException(err.message)
78-
bugsnag.notify(err)
79-
console.warn(err)
80-
buildings = fallbackBuildingHours
81-
}
82-
69+
this.setState(() => ({loading: true}))
70+
let {data: buildings} = await fetchJson(githubBaseUrl).catch(err => {
71+
reportNetworkProblem(err)
72+
return defaultData
73+
})
8374
if (process.env.NODE_ENV === 'development') {
84-
buildings = fallbackBuildingHours
75+
buildings = defaultData.data
8576
}
86-
87-
this.setState({
77+
this.setState(() => ({
8878
loading: false,
8979
buildings: groupBuildings(buildings),
9080
now: moment.tz(CENTRAL_TZ),
91-
})
81+
}))
9282
}
9383

9484
render() {
9585
if (this.state.error) {
96-
return <NoticeView text={'Error: ' + this.state.error.message} />
86+
return <NoticeView text={`Error: ${this.state.error.message}`} />
9787
}
9888

9989
return (

0 commit comments

Comments
 (0)