Skip to content

Commit

Permalink
theme: Fix latent bug with colors not updating on rerender.
Browse files Browse the repository at this point in the history
Compute theme-dependent styles in the render method.

As foreshadowed in a previous commit that touched
src/common/Screen.js, fix the remaining latent bugs where the
placement of theme-dependent styles in a class initializer, instead
of the `render` method, meant that these styles don't live-update
when the theme changes, unless the component is unmounted and a new
instance is created to replace it.

The `key={theme}` hack accomplished this replacement, so this bug
hasn't been visible. But that hack will disappear in an upcoming
commit, so this change needs to be made before then.

These bugs were introduced in

51dd1b3: LineSeparator.js
a4e0f23: OptionButton.js and OptionRow.js
f6ddc2d: OptionDivider.js
  • Loading branch information
Chris Bobbe authored and gnprice committed May 1, 2020
1 parent c305ef5 commit dfd9cda
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 15 deletions.
5 changes: 3 additions & 2 deletions src/common/LineSeparator.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ export default class LineSeparator extends PureComponent<{||}> {
styles = {
lineSeparator: {
height: 1,
backgroundColor: this.context.cardColor,
margin: 4,
},
};

render() {
return <View style={this.styles.lineSeparator} />;
return (
<View style={[this.styles.lineSeparator, { backgroundColor: this.context.cardColor }]} />
);
}
}
9 changes: 3 additions & 6 deletions src/common/OptionButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ export default class OptionButton extends PureComponent<Props> {
context: ThemeColors;

styles = {
icon: {
...styles.settingsIcon,
color: this.context.color,
},
icon: styles.settingsIcon,
};

render() {
Expand All @@ -32,10 +29,10 @@ export default class OptionButton extends PureComponent<Props> {
return (
<Touchable onPress={onPress}>
<View style={styles.listItem}>
{!!Icon && <Icon size={18} style={this.styles.icon} />}
{!!Icon && <Icon size={18} style={[this.styles.icon, { color: this.context.color }]} />}
<Label text={label} />
<View style={styles.rightItem}>
<IconRight size={18} style={this.styles.icon} />
<IconRight size={18} style={[this.styles.icon, { color: this.context.color }]} />
</View>
</View>
</Touchable>
Expand Down
3 changes: 1 addition & 2 deletions src/common/OptionDivider.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ export default class OptionDivider extends PureComponent<{||}> {
styles = {
divider: {
borderBottomWidth: 1,
borderBottomColor: this.context.dividerColor,
},
};

render() {
return <View style={this.styles.divider} />;
return <View style={[this.styles.divider, { borderBottomColor: this.context.dividerColor }]} />;
}
}
7 changes: 2 additions & 5 deletions src/common/OptionRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@ export default class OptionRow extends PureComponent<Props> {
context: ThemeColors;

styles = {
icon: {
...styles.settingsIcon,
color: this.context.color,
},
icon: styles.settingsIcon,
};

render() {
const { label, value, onValueChange, style, Icon } = this.props;

return (
<View style={[styles.listItem, style]}>
{!!Icon && <Icon size={18} style={this.styles.icon} />}
{!!Icon && <Icon size={18} style={[this.styles.icon, { color: this.context.color }]} />}
<Label text={label} style={styles.flexed} />
<View style={styles.rightItem}>
<ZulipSwitch value={value} onValueChange={onValueChange} />
Expand Down

0 comments on commit dfd9cda

Please sign in to comment.