Skip to content

Commit f6ff91a

Browse files
BigHeadCreationsgnprice
authored andcommitted
theme: Provide name in ThemeData; switch to context in MessageList
Change the way `MessageList` updates its theme. It now gets its ThemeData information from the `ThemeContext` instead of its selector props. This is helpful if the theme ever changes, by causing the component to re-render rather than get the colors out of sync. That currently isn't possible while a `MessageList` is mounted, because the theme can only change by navigating to the settings screen; but it will become possible with #4009 via the OS-level theme changing.
1 parent 382556f commit f6ff91a

File tree

3 files changed

+23
-16
lines changed

3 files changed

+23
-16
lines changed

docs/architecture/react.md

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -141,20 +141,19 @@ doc](https://reactjs.org/docs/context.html)):
141141
> [...])
142142
> is not subject to the shouldComponentUpdate method
143143
144-
We also confirmed this behavior experimentally, in a 2020 version of
145-
`MessageList` which used `ThemeContext` to get the theme colors.
146-
(Since 1ba871910, `MessageList` uses a transparent background and so
147-
doesn't need the theme; since 4fa2418b8 it doesn't mention
148-
`ThemeContext`.) That component re-`render`ed when the theme changed,
149-
*even though its `shouldComponentUpdate` always returned `false`*.
150-
This didn't cause a live problem because the UI doesn't
151-
allow changing the theme while a `MessageList` is in the navigation
152-
stack. If it were possible, it would be a concern: setting a short
153-
interval to automatically toggle the theme, we see that the message
154-
list's color scheme changes as we'd want it to, but we also see the
155-
bad effects that `shouldComponentUpdate` returning `false` is meant to
156-
prevent: losing the scroll position, mainly (but also, we expect,
157-
discarding the image cache, etc.).
144+
Concretely, this means that our `MessageList` component updates
145+
(re-`render`s) when the theme changes, since it's a `ThemeContext`
146+
consumer, *even though its `shouldComponentUpdate` always returns
147+
`false`*. This generally isn't a problem because the UI for
148+
changing our own theme setting can't appear while a `MessageList` is
149+
in the navigation stack; so the theme can change only once we have
150+
#4009, via the OS-level theme changing (either automatically on
151+
schedule, or because the user changed it in system settings.) When
152+
this does happen we see that the message list's color scheme changes
153+
as we'd want it to, but we also see the bad effects that
154+
`shouldComponentUpdate` returning `false` is meant to prevent:
155+
losing the scroll position, mainly (but also, we expect, discarding
156+
the image cache, etc.).
158157

159158
### The exception: `MessageList`
160159

src/styles/theme.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { Context } from 'react';
55
import type { ThemeName } from '../reduxTypes';
66

77
export type ThemeData = {|
8+
themeName: ThemeName,
89
color: string,
910
backgroundColor: string,
1011
cardColor: string,
@@ -13,6 +14,7 @@ export type ThemeData = {|
1314

1415
export const themeData: {| [name: ThemeName | 'light']: ThemeData |} = {
1516
night: {
17+
themeName: 'night',
1618
color: 'hsl(210, 11%, 85%)',
1719
backgroundColor: 'hsl(212, 28%, 18%)',
1820
cardColor: 'hsl(212, 31%, 21%)',
@@ -21,6 +23,7 @@ export const themeData: {| [name: ThemeName | 'light']: ThemeData |} = {
2123
dividerColor: 'hsla(0, 0%, 100%, 0.12)',
2224
},
2325
light: {
26+
themeName: 'default',
2427
color: 'hsl(0, 0%, 20%)',
2528
backgroundColor: 'white',
2629
cardColor: 'hsl(0, 0%, 97%)',

src/webview/MessageList.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import type {
1616
EditMessage,
1717
} from '../types';
1818
import { assumeSecretlyGlobalState } from '../reduxTypes';
19+
import type { ThemeData } from '../styles';
20+
import { ThemeContext } from '../styles';
1921
import { connect } from '../react-redux';
2022
import {
2123
getCurrentTypingUsers,
@@ -123,6 +125,9 @@ const webviewAssetsUrl = new URL('webview/', assetsUrl);
123125
const baseUrl = new URL('index.html', webviewAssetsUrl);
124126

125127
class MessageListInner extends React.Component<Props> {
128+
static contextType = ThemeContext;
129+
context: ThemeData;
130+
126131
webviewRef = React.createRef<React$ElementRef<typeof WebView>>();
127132
sendInboundEventsIsReady: boolean;
128133
unsentInboundEvents: WebViewInboundEvent[] = [];
@@ -176,10 +181,10 @@ class MessageListInner extends React.Component<Props> {
176181
const contentHtml = messageListElementsForShownMessages
177182
.map(element => messageListElementHtml({ backgroundData, element, _ }))
178183
.join('');
179-
const { auth, theme } = backgroundData;
184+
const { auth } = backgroundData;
180185
const html: string = getHtml(
181186
contentHtml,
182-
theme,
187+
this.context.themeName,
183188
{
184189
scrollMessageId: initialScrollMessageId,
185190
auth,

0 commit comments

Comments
 (0)