Skip to content

Commit 0bafcae

Browse files
chrisbobbegnprice
authored andcommitted
offline notice: Make into an overlying surface that covers the top inset
This seems more Material Design-like. I've never liked the old style where we'd cut through an existing surface (such as an app bar) with a gray strip. Soon, we'll animate the notice's entrance and exit so that it slides down and up. On a modern iPhone, this now covers the top inset with gray, but without taking any more vertical space in the "safe area" (the area not covered by the insets) than before. We make the change like this: - Centralize the notice into a single UI component rendered by the new OfflineNoticeProvider, with similar style except it now has a higher z-index, it's absolutely positioned, and it uses SafeAreaView to occupy the top inset. - Everywhere we used to render an OfflineNotice, instead render an OfflineNoticePlaceholder, which will scoot content downward so it doesn't get obscured when the notice appears. The OfflineNoticePlaceholder asks its callers to place it as the topmost item in the Y direction just under the top-inset padding. We already gave that placement to all the `OfflineNotice`s in a recent commit, so that's fine. We add the accessibility props so that we don't regress on the ability to check for the connectivity state by focusing the notice. If we didn't make the view unfocusable when off-screen, I assume a screen-reader could still focus it, and it would say "offline", which would be confusing. I tested at this commit on my iPhone (VoiceOver on iOS 15.6, iPhone 13 Pro) and the office Android device (Voice Assistant on Android 9, Samsung Galaxy S9), and it worked as intended.
1 parent f522130 commit 0bafcae

File tree

12 files changed

+294
-119
lines changed

12 files changed

+294
-119
lines changed

src/ZulipMobile.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import CompatibilityChecker from './boot/CompatibilityChecker';
1616
import AppEventHandlers from './boot/AppEventHandlers';
1717
import { initializeSentry } from './sentry';
1818
import ZulipSafeAreaProvider from './boot/ZulipSafeAreaProvider';
19+
import { OfflineNoticeProvider } from './boot/OfflineNoticeProvider';
1920

2021
initializeSentry();
2122

@@ -55,9 +56,11 @@ export default function ZulipMobile(): Node {
5556
<AppEventHandlers>
5657
<TranslationProvider>
5758
<ThemeProvider>
58-
<ActionSheetProvider>
59-
<ZulipNavigationContainer />
60-
</ActionSheetProvider>
59+
<OfflineNoticeProvider>
60+
<ActionSheetProvider>
61+
<ZulipNavigationContainer />
62+
</ActionSheetProvider>
63+
</OfflineNoticeProvider>
6164
</ThemeProvider>
6265
</TranslationProvider>
6366
</AppEventHandlers>

src/account-info/ProfileScreen.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { getOwnUser } from '../users/userSelectors';
1919
import { getIdentity } from '../account/accountsSelectors';
2020
import { useNavigation } from '../react-navigation';
2121
import { showConfirmationDialog } from '../utils/info';
22-
import OfflineNotice from '../common/OfflineNotice';
22+
import { OfflineNoticePlaceholder } from '../boot/OfflineNoticeProvider';
2323

2424
const styles = createStyleSheet({
2525
buttonRow: {
@@ -129,7 +129,7 @@ export default function ProfileScreen(props: Props): Node {
129129

130130
return (
131131
<SafeAreaView mode="padding" edges={['top']} style={{ flex: 1 }}>
132-
<OfflineNotice />
132+
<OfflineNoticePlaceholder />
133133
<ScrollView>
134134
<AccountDetails user={ownUser} />
135135
<AwayStatusSwitch />

src/boot/AppEventHandlers.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ class AppEventHandlersInner extends PureComponent<Props> {
122122
handleInitialShare();
123123

124124
NetInfo.configure({
125-
// This is the default, as of 6.0.0, but `OfflineNotice` depends on this
126-
// value being stable.
125+
// This is the default, as of 6.0.0, but `OfflineNoticeProvider`
126+
// depends on this value being stable.
127127
reachabilityRequestTimeout: 15 * 1000,
128128
});
129129
this.netInfoDisconnectCallback = NetInfo.addEventListener(this.handleConnectivityChange);

src/boot/OfflineNoticeProvider.js

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
// @flow strict-local
2+
import React, { useCallback, useContext, useEffect, useMemo, useState } from 'react';
3+
import type { Node } from 'react';
4+
import { View } from 'react-native';
5+
import NetInfo from '@react-native-community/netinfo';
6+
import { SafeAreaView } from 'react-native-safe-area-context';
7+
import type { ViewProps } from 'react-native/Libraries/Components/View/ViewPropTypes';
8+
import type { DimensionValue } from 'react-native/Libraries/StyleSheet/StyleSheetTypes';
9+
10+
import * as logging from '../utils/logging';
11+
import { useGlobalSelector } from '../react-redux';
12+
import { getGlobalSession, getGlobalSettings } from '../directSelectors';
13+
import { useHasStayedTrueForMs } from '../reactUtils';
14+
import type { JSONableDict } from '../utils/jsonable';
15+
import { createStyleSheet } from '../styles';
16+
import ZulipTextIntl from '../common/ZulipTextIntl';
17+
import type { ViewStylePropWithout } from '../reactNativeUtils';
18+
import ZulipStatusBar from '../common/ZulipStatusBar';
19+
import type { ThemeName } from '../reduxTypes';
20+
21+
function useShouldShowUncertaintyNotice(): boolean {
22+
const isOnline = useGlobalSelector(state => getGlobalSession(state).isOnline);
23+
24+
const result = useHasStayedTrueForMs(
25+
// See note in `SessionState` for what this means.
26+
isOnline === null,
27+
28+
// A decently long time, much longer than it takes to send `true` or
29+
// `false` over the RN bridge.
30+
//
31+
// Also, one second longer than what we set for
32+
// `reachabilityRequestTimeout` in NetInfo's config (15s), which is the
33+
// longest `isOnline` can be `null` on iOS in an expected case. For
34+
// details, see the comment where we dispatch the action to change
35+
// `isOnline`.
36+
//
37+
// If this time passes and `isOnline` is still `null`, we should treat
38+
// it as a bug and investigate.
39+
16 * 1000,
40+
);
41+
42+
useEffect(() => {
43+
if (result) {
44+
NetInfo.fetch().then(state => {
45+
logging.warn(
46+
'Failed to determine Internet reachability in a reasonable time',
47+
// `state`, being inexact, might have unknown properties that
48+
// aren't JSONable. Hopefully Sentry would just drop parts that
49+
// aren't JSONable instead of panicking or dropping everything.
50+
// $FlowFixMe[incompatible-cast]
51+
(state: JSONableDict),
52+
);
53+
});
54+
}
55+
}, [result]);
56+
57+
return result;
58+
}
59+
60+
const OfflineNoticeContext = React.createContext({
61+
isNoticeVisible: false,
62+
noticeContentAreaHeight: 0,
63+
});
64+
65+
type ProviderProps = {|
66+
+children: Node,
67+
|};
68+
69+
const backgroundColorForTheme = (theme: ThemeName): string =>
70+
// TODO(redesign): Choose these more intentionally; these are just the
71+
// semitransparent HALF_COLOR flattened with themeData.backgroundColor.
72+
// See https://github.com/zulip/zulip-mobile/pull/5491#issuecomment-1282859332
73+
theme === 'default' ? '#bfbfbf' : '#50565e';
74+
75+
/**
76+
* Shows a notice if the app is working in offline mode.
77+
*
78+
* Shows a different notice if we've taken longer than we expect to
79+
* determine Internet reachability. IOW, if the user sees this, there's a
80+
* bug.
81+
*
82+
* Shows nothing if the Internet is reachable.
83+
*
84+
* The notice is a banner at the top of the screen. All screens should
85+
* render a OfflineNoticePlaceholder so that banner doesn't hide any of the
86+
* screen's content; see there for details.
87+
*/
88+
export function OfflineNoticeProvider(props: ProviderProps): Node {
89+
const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
90+
const isOnline = useGlobalSelector(state => getGlobalSession(state).isOnline);
91+
const shouldShowUncertaintyNotice = useShouldShowUncertaintyNotice();
92+
const isNoticeVisible = isOnline === false || shouldShowUncertaintyNotice;
93+
94+
const styles = useMemo(
95+
() =>
96+
createStyleSheet({
97+
flex1: { flex: 1 },
98+
noticeSurface: {
99+
position: 'absolute',
100+
101+
// Whether the notice is visible or tucked away above the window.
102+
...(isNoticeVisible ? { top: 0 } : { bottom: '100%' }),
103+
104+
zIndex: 1,
105+
width: '100%',
106+
107+
// If changing, also change the status bar color in
108+
// OfflineNoticePlaceholder.
109+
backgroundColor: backgroundColorForTheme(theme),
110+
111+
justifyContent: 'center',
112+
alignItems: 'center',
113+
},
114+
noticeContentArea: {
115+
width: '100%',
116+
alignItems: 'center',
117+
justifyContent: 'center',
118+
paddingVertical: 4,
119+
},
120+
noticeText: { fontSize: 14 },
121+
}),
122+
[isNoticeVisible, theme],
123+
);
124+
125+
/**
126+
* Our best guess at the computed height of the content area in the
127+
* layout caused by this render.
128+
*
129+
* Pass this to OfflineNoticePlaceholder so it reserves the right amount
130+
* of height for the notice when the notice is visible.
131+
*
132+
* If wrong, expect OfflineNoticePlaceholder to visibly have the wrong
133+
* height for a moment but then recover quickly.
134+
*/
135+
// It's the computed height from the *last* render (or zero if this is the
136+
// first render), using the onLayout prop. So, to make a good guess, we
137+
// try to avoid changes to the computed height:
138+
// - When the notice is offscreen, we set the text to match the more
139+
// likely message when the notice comes onscreen, namely the "offline"
140+
// message. That means it'll say "offline" when the state is actually
141+
// "online", but that's OK because it'll be offscreen.
142+
// - When the notice is onscreen, it'll show either an "offline" message
143+
// or an "uncertain" message. It's possible but unlikely that just one
144+
// message would wrap onto two lines, inviting errors by 0.5x or 2x.
145+
// To keep that possibility in check, we don't make either message
146+
// very detailed, so they'd be similar in length in any language, and
147+
// hopefully both short enough to stay on one line, even with a
148+
// large-ish system font setting.
149+
const [noticeContentAreaHeight, setNoticeContentAreaHeight] = useState(0);
150+
151+
const handleNoticeContentAreaLayoutChange = useCallback(event => {
152+
setNoticeContentAreaHeight(event.nativeEvent.layout.height);
153+
}, []);
154+
155+
const contextValue = useMemo(
156+
() => ({ isNoticeVisible, noticeContentAreaHeight }),
157+
[isNoticeVisible, noticeContentAreaHeight],
158+
);
159+
160+
return (
161+
<View style={styles.flex1}>
162+
<SafeAreaView
163+
mode="padding"
164+
edges={['top', 'right', 'left']}
165+
style={styles.noticeSurface}
166+
{
167+
// Just as we do in the visual UI, offer screen readers a way to
168+
// check the connectivity state on demand: the notice will either
169+
// be present, with explanatory text, or absent. To make it
170+
// "absent" for screen readers, we make this view and its children
171+
// unfocusable.
172+
...(isNoticeVisible
173+
? {
174+
// Group descendants into a single selectable component. Its
175+
// text will automatically be the notice text (confirmed
176+
// experimentally), so no need for an `accessibilityLabel`.
177+
accessible: true,
178+
}
179+
: {
180+
accessible: false,
181+
accessibilityElementsHidden: true,
182+
importantForAccessibility: 'no-hide-descendants',
183+
})
184+
}
185+
>
186+
<View onLayout={handleNoticeContentAreaLayoutChange} style={styles.noticeContentArea}>
187+
<ZulipTextIntl
188+
style={styles.noticeText}
189+
text={
190+
shouldShowUncertaintyNotice
191+
? 'Please check your Internet connection'
192+
: 'No Internet connection'
193+
}
194+
/>
195+
</View>
196+
</SafeAreaView>
197+
<View style={styles.flex1}>
198+
<OfflineNoticeContext.Provider value={contextValue}>
199+
{props.children}
200+
</OfflineNoticeContext.Provider>
201+
</View>
202+
</View>
203+
);
204+
}
205+
206+
type PlaceholderProps = $ReadOnly<{|
207+
...ViewProps,
208+
style?: ViewStylePropWithout<{|
209+
// Let this component do its job; don't mess with its height.
210+
height: DimensionValue,
211+
minHeight: DimensionValue,
212+
maxHeight: DimensionValue,
213+
|}>,
214+
|}>;
215+
216+
/**
217+
* Empty View that expands from 0 height to give room to the offline notice.
218+
*
219+
* On every screen, render one of these on the surface that occupies the top
220+
* of the screen in the Y direction (e.g., an app bar), inside that
221+
* surface's top-inset padding.
222+
*
223+
* The offline notice will be overlaid on top of that surface in the Z
224+
* direction, blocking part of it from view including its top-inset padding.
225+
* Use this placeholder to push the content on the underlying surface
226+
* downward in the Y direction, just enough so it doesn't get hidden by the
227+
* offline notice's content and only when the offline notice is actually
228+
* onscreen.
229+
*
230+
* On Android, where the app doesn't draw underneath the status bar, this
231+
* also colors the status bar's background to match the notice.
232+
*
233+
* Must have the OfflineNoticeProvider above it in the tree.
234+
*/
235+
export function OfflineNoticePlaceholder(props: PlaceholderProps): Node {
236+
const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
237+
const { style: callerStyle } = props;
238+
239+
const { isNoticeVisible, noticeContentAreaHeight } = useContext(OfflineNoticeContext);
240+
241+
const style = useMemo(
242+
() => [
243+
{
244+
height: isNoticeVisible ? noticeContentAreaHeight : 0,
245+
width: '100%',
246+
backgroundColor: 'transparent',
247+
},
248+
callerStyle,
249+
],
250+
[isNoticeVisible, noticeContentAreaHeight, callerStyle],
251+
);
252+
253+
return (
254+
<>
255+
{
256+
// On Android, have the notice's background color extend through the
257+
// status bar. We do this here instead of in OfflineNoticeProvider
258+
// so our instruction to the status bar doesn't get clobbered by
259+
// another instruction more leafward than OfflineNoticeProvider.
260+
isNoticeVisible && (
261+
<ZulipStatusBar
262+
// Should match the notice's surface; see OfflineNoticeProvider.
263+
backgroundColor={backgroundColorForTheme(theme)}
264+
/>
265+
)
266+
}
267+
<View style={style} />
268+
</>
269+
);
270+
}

src/common/FullScreenLoading.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context';
77
import { BRAND_COLOR, createStyleSheet } from '../styles';
88
import LoadingIndicator from './LoadingIndicator';
99
import ZulipStatusBar from './ZulipStatusBar';
10-
import OfflineNotice from './OfflineNotice';
10+
import { OfflineNoticePlaceholder } from '../boot/OfflineNoticeProvider';
1111

1212
const componentStyles = createStyleSheet({
1313
center: {
@@ -36,7 +36,7 @@ export default function FullScreenLoading(props: Props): Node {
3636
backgroundColor: BRAND_COLOR,
3737
}}
3838
/>
39-
<OfflineNotice />
39+
<OfflineNoticePlaceholder />
4040
<LoadingIndicator color="black" size={80} showLogo />
4141
</View>
4242
</>

0 commit comments

Comments
 (0)