diff --git a/docs/architecture/safe-areas.md b/docs/architecture/safe-areas.md new file mode 100644 index 00000000000..f881863d14f --- /dev/null +++ b/docs/architecture/safe-areas.md @@ -0,0 +1,146 @@ +# Handling "safe areas" in the visual layout + +Some devices, like the iPhone X, have notches and native UI elements that +can overlap important content if we're not careful. We generally want the +boring, background part of elements to occupy the insets, and nothing else. +(We do let the lightbox image extend through the insets. This is a common +design choice, and the user can always zoom out to see what was hidden.) + +We generally follow React Navigation's recommendations in their +[doc](https://github.com/zulip/react-native) on safe areas. React +Navigation's built-in UI elements help us by handling the insets in some +places. But the available built-ins don't solve the entire problem, and we +also [choose not to +use](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-nav.3A.20headers/near/1126189) +some of them because of their questionable design. Our own custom components +have to be part of the solution. + +We use +[`react-native-safe-area-context`](https://www.npmjs.com/package/react-native-safe-area-context), +on React Navigation's recommendation. + +## Rules for handling safe areas + +1. As part of every component's interface, we should include how it's meant + to interact with the safe area. + - Most components aren't aware of the safe area: they don't do anything + about it, and they assume they are rendered entirely inside it. So + this is the default, and we generally won't mention it in the jsdoc in + those cases. + - Some components expect to occupy the horizontal insets, some the + bottom inset, and some the top inset. (In principle there could be + other valid combinations.) For example, a header component should + occupy at least the top inset so that the header's background extends + to the top edge of the screen. This should be announced in the jsdoc, + saying why the component occupies the inset(s). +2. If a component wants to occupy any of the insets, it should generally do + so with a `SafeAreaView`, with `"padding"` for `mode`, and with the + intended `edges`. We prefer this component over `useSafeAreaInsets` and + friends, where possible, because it does the layout calculations + natively. If we have to look at inset dimensions in JavaScript, they + couldn't be up-to-date because they've been carried over React Native's + asynchronous bridge, and there will be a flicker when the insets change + (e.g., on orientation changes). +3. If a React component is meant to occupy any insets, then its layout + parent should adapt to allow it to do so, so that the inset distance + isn't covered twice. This generally means omitting a padding + `SafeAreaView` wrapper from the parent, if present, putting one in the + child, and adjusting the wrapper's other children as necessary to + compensate (e.g., by giving them `SafeAreaView`s, preferably with margin + instead of padding, to show they don't care what fills the space). There + are two reasons the parent should adapt: + - When it's a case where the child has a stronger claim to choose what + goes in the insets than the parent does, e.g., to extend its own + background to the edge of the screen. This comes up a lot with + list-item elements that want to extend their backgrounds to the left + and right edges of the screen, while keeping their meaningful content + within the safe area. + - On iOS, at least, a `SafeAreaView` doesn't change what padding/margin + it applies based on its position relative to the actual insets or + other `SafeAreaView`s. It just statically applies the inset distance, + wherever it is; a `SafeAreaView` nested in another `SafeAreaView` + will duplicate the padding/margin. So it's not convenient to make + components agnostic to whether an ancestor occupies the insets, and + we have to keep careful track of where we handle the insets. See the + "iOS" section for more on the relevant constraints here. + + +## iOS + +We were surprised to +[find](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1230454) +the following [hypothesis](https://github.com/zulip/zulip-mobile/pull/4893#discussion_r668412622) wasn't true: + +> I think if you make a `SafeAreaView` at a place where it's actually completely inside the safe area -- for example, because it's nested other `SafeAreaView`s that provided the appropriate padding on each side -- then it has no effect. + +It seemed like it should be true, especially because that's how the relevant +iOS API works, namely [the `safeAreaInsets` property on +UIView](https://developer.apple.com/documentation/uikit/uiview/2891103-safeareainsets?language=objc). +But Greg +[found](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1230523), +"[F]or some reason it's [not calling that method on the relevant view +itself](https://github.com/th3rdwave/react-native-safe-area-context/blob/cd8dd60d035a44c22459b2c890e6512e5796396e/ios/SafeAreaView/RNCSafeAreaView.m#L76), +i.e. the `RNCSafeAreaView`. Instead, it [walks up the tree to the enclosing +`RNCSafeAreaProvider`](https://github.com/th3rdwave/react-native-safe-area-context/blob/cd8dd60d035a44c22459b2c890e6512e5796396e/ios/SafeAreaView/RNCSafeAreaView.m#L88-L94), +and asks what *that* view's `safeAreaInsets` is. Which is... just the wrong +question." + +So that explains why every `SafeAreaView` uses the same values for +margin/padding at a given time. They're not getting the values from their +own position, but from the position of their shared `SafeAreaProvider`. + +We +[tried](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1231222) +changing it to get `safeAreaInsets` from the self instead of the provider. +This seemed to make the above-quoted hypothesis true, which was great—except +we noticed a bug, which is plausibly the reason for not doing it this way. + +To understand the bug, note that the "self" approach will predictably affect +how `SafeAreaView`s behave when they're involved in slide animations near +the edges of the screen. For example, if the left edge of a `SafeAreaView` +is sliding left toward the edge of the screen, and it has entered the left +inset, then its left padding should increase with each frame so that the +content it's guarding will stay still within the safe area. See +[videos](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1231279) +of this happening with a navigation transition. + +We do see this happening approximately, but not perfectly. Greg +[describes](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1231377) +after seeing those videos, + +> It looks like the whole content of the page has some jank -- specifically +> some jitter on the left edge, in the last moments before it lands where +> it's going. I think that probably indicates some lack of synchronicity. +> Like it's looking up what the inset is, and it gets the answer as of right +> now… but then it uses that answer for the next frame and the next. And +> when there's an animation happening, the answer is changing, so the answer +> is wrong by the time it's laying out those later frames. + +Greg then +[found](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1231419) +a piece of code in the library's iOS native layer that "does look to me kind +of suspiciously like it's setting some data that the RN layout engine will +use for the next frame -- not like it's using an API where it gets asked to +provide some information on each frame." + +This lack of synchronicity (even within the native layer—this is independent +of the problem with `useSafeAreaInsets` and friends, due to RN's +asynchronous bridge) might be due to a fundamental limitation in React +Native. + +Anyway, we don't want that jitter, and this is plausibly explains why the +library has each `SafeAreaView` ask for its provider's, not its own, idea of +the necessary padding/margin. + +This bug is probably a factor in how rough things look when you change the +device orientation on iOS. + +## Android + +We've never seen a case where an Android device has nonzero insets. As far +as we've seen, the whole app seems to occupy a clear rectangle given to it +by the system. It's below the status bar, and there are no overlapping +notches or native UI. When this is the case, each `SafeAreaView` will +appropriately act as a plain `View`. See +[discussion](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-native-safe-area-context.20padding/near/1230471) +for more observations on Android. diff --git a/docs/style.md b/docs/style.md index 4be1ccde437..ef8c14990db 100644 --- a/docs/style.md +++ b/docs/style.md @@ -556,6 +556,15 @@ code where we're using a given aspect of the `display_recipient` semantics, which makes refactoring easier. + +
+ +### Layout principles + +See our [architecture doc on handling safe +areas](architecture/safe-areas.md) in the visual layout. + +
## WebView: HTML, CSS, JS diff --git a/src/account-info/AccountDetailsScreen.js b/src/account-info/AccountDetailsScreen.js index 9ffdfa66743..5c048c6ad92 100644 --- a/src/account-info/AccountDetailsScreen.js +++ b/src/account-info/AccountDetailsScreen.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import React, { PureComponent } from 'react'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { RouteProp } from '../react-navigation'; import type { AppNavigationProp } from '../nav/AppNavigator'; @@ -38,6 +39,11 @@ type Props = $ReadOnly<{| ...SelectorProps, |}>; +/** + * Details about a selected user. + * + * Pads the horizontal insets with its background. + */ class AccountDetailsScreen extends PureComponent { handleChatPress = () => { const { user, dispatch } = this.props; @@ -56,16 +62,18 @@ class AccountDetailsScreen extends PureComponent { return ( - - {!isActive && ( - ); } diff --git a/src/account-info/AwayStatusSwitch.js b/src/account-info/AwayStatusSwitch.js index 2c31f41deac..9fe80959061 100644 --- a/src/account-info/AwayStatusSwitch.js +++ b/src/account-info/AwayStatusSwitch.js @@ -13,6 +13,8 @@ type Props = $ReadOnly<{||}>; * This is a stand-alone component that: * * retrieves the current user's `user status` data and presents it * * allows by switching it to control the `away` status + * + * Needs to occupy the horizontal insets because `SwitchRow` does. */ export default function AwayStatusSwitch(props: Props): Node { const awayStatus = useSelector(getSelfUserAwayStatus); diff --git a/src/account-info/ProfileScreen.js b/src/account-info/ProfileScreen.js index 8ce9560845b..601ff9de353 100644 --- a/src/account-info/ProfileScreen.js +++ b/src/account-info/ProfileScreen.js @@ -1,7 +1,8 @@ /* @flow strict-local */ import React, { useContext } from 'react'; import type { Node } from 'react'; -import { ScrollView, View, Alert } from 'react-native'; +import { ScrollView, Alert } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import { TranslationContext } from '../boot/TranslationProvider'; import type { RouteProp } from '../react-navigation'; @@ -102,21 +103,26 @@ type Props = $ReadOnly<{| * It does not have a "Send private message" but has "Switch account" and "Log out" buttons. * * The user can still open `AccountDetails` on themselves via the (i) icon in a chat screen. + * + * Needs to occupy the horizontal insets because the away-status switch + * does. */ export default function ProfileScreen(props: Props): Node { const ownUser = useSelector(getOwnUser); return ( - + + + - + - - + + - + ); } diff --git a/src/account/AccountPickScreen.js b/src/account/AccountPickScreen.js index 6732d91ecd9..b5c9c9da612 100644 --- a/src/account/AccountPickScreen.js +++ b/src/account/AccountPickScreen.js @@ -3,6 +3,7 @@ import React, { useContext, useCallback } from 'react'; import type { Node } from 'react'; import { Alert } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import * as api from '../api'; import { TranslationContext } from '../boot/TranslationProvider'; @@ -27,6 +28,11 @@ type Props = $ReadOnly<{| route: RouteProp<'account-pick', void>, |}>; +/** + * A screen to choose an account the app knows about, to make it active. + * + * Pads the horizontal insets with its background. + */ export default function AccountPickScreen(props: Props): Node { const { navigation } = props; const accounts = useSelector(getAccountStatuses); @@ -85,21 +91,23 @@ export default function AccountPickScreen(props: Props): Node { canGoBack={navigation.canGoBack()} shouldShowLoadingBanner={false} > - - {accounts.length === 0 && } - - - { - NavigationService.dispatch(navigateToRealmInputScreen()); - }} - /> - + + + {accounts.length === 0 && } + + + { + NavigationService.dispatch(navigateToRealmInputScreen()); + }} + /> + + ); } diff --git a/src/autocomplete/EmojiAutocomplete.js b/src/autocomplete/EmojiAutocomplete.js index c00aaa4ecb3..ae62c08d850 100644 --- a/src/autocomplete/EmojiAutocomplete.js +++ b/src/autocomplete/EmojiAutocomplete.js @@ -34,6 +34,9 @@ export default function EmojiAutocomplete(props: Props): Node { data={emojiNames.slice(0, MAX_CHOICES)} keyExtractor={item => item.name} renderItem={({ item }) => ( + // TODO: Make and use a new emoji-item component with no padding + // for the insets. The rows' content should be bounded by the + // popup, which renders within the safe area. item.stream_id.toString()} renderItem={({ item }) => ( + // TODO: Make and use a new stream-item component with no padding + // for the insets. The rows' content should be bounded by the + // popup, which renders within the safe area. + @@ -134,13 +152,18 @@ export default function ChatScreen(props: Props): Node { return ; } else { return ( - + // TODO: Keep the meaningful content within the safe areas, but + // let the background elements extend through the insets all the + // way to the left and right of the screen. + + + ); } })()} diff --git a/src/chat/UnreadNotice.js b/src/chat/UnreadNotice.js index 01d898692c4..bc10d22f625 100644 --- a/src/chat/UnreadNotice.js +++ b/src/chat/UnreadNotice.js @@ -1,13 +1,13 @@ /* @flow strict-local */ import React from 'react'; -import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { Narrow } from '../types'; import { createStyleSheet } from '../styles'; import { useSelector } from '../react-redux'; import { getUnreadCountForNarrow } from '../selectors'; -import { Label, RawLabel } from '../common'; +import { Label } from '../common'; import MarkAsReadButton from './MarkAsReadButton'; import AnimatedScaleComponent from '../animation/AnimatedScaleComponent'; @@ -16,19 +16,13 @@ const styles = createStyleSheet({ paddingHorizontal: 8, paddingVertical: 4, backgroundColor: 'hsl(232, 89%, 78%)', - flexDirection: 'row', - justifyContent: 'space-between', overflow: 'hidden', }, - unreadTextWrapper: { + safeAreaWrapper: { flexDirection: 'row', + justifyContent: 'space-between', alignItems: 'center', }, - unreadNumber: { - fontSize: 14, - color: 'white', - paddingRight: 4, - }, unreadText: { fontSize: 14, color: 'white', @@ -39,20 +33,31 @@ type Props = $ReadOnly<{| narrow: Narrow, |}>; +/** + * Says how many unread messages are in the narrow. + * + * Pads the left and right insets with its background. + */ export default function UnreadNotice(props: Props) { const { narrow } = props; const unreadCount = useSelector(state => getUnreadCountForNarrow(state, narrow)); return ( 0} style={styles.unreadContainer}> - - + - + + ); } diff --git a/src/common/FullScreenLoading.js b/src/common/FullScreenLoading.js index e1ca1bc7b6f..035b84dedc6 100644 --- a/src/common/FullScreenLoading.js +++ b/src/common/FullScreenLoading.js @@ -2,7 +2,6 @@ import React from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; -import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { BRAND_COLOR, createStyleSheet } from '../styles'; // eslint-disable-next-line import/no-useless-path-segments @@ -23,18 +22,10 @@ type Props = $ReadOnly<{||}>; * Meant to be used to cover the whole screen. */ export default function FullScreenLoading(props: Props): Node { - const insets = useSafeAreaInsets(); - return ( <> - diff --git a/src/common/NestedNavRow.js b/src/common/NestedNavRow.js index d1526386c2b..bb2a11773fa 100644 --- a/src/common/NestedNavRow.js +++ b/src/common/NestedNavRow.js @@ -2,6 +2,7 @@ import React, { useContext } from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import Label from './Label'; import Touchable from './Touchable'; @@ -22,6 +23,8 @@ type Props = $ReadOnly<{| * * Shows a right-facing arrow to indicate its purpose. If you need a * selectable option row instead, use `SelectableOptionRow`. + * + * Pads the horizontal insets with its background. */ export default function NestedNavRow(props: Props): Node { const { label, onPress, Icon } = props; @@ -30,13 +33,13 @@ export default function NestedNavRow(props: Props): Node { return ( - + {!!Icon && } + ); } diff --git a/src/common/Popup.js b/src/common/Popup.js index de7a084903e..5bb9ccac39c 100644 --- a/src/common/Popup.js +++ b/src/common/Popup.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React, { useContext } from 'react'; import type { Node } from 'react'; -import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import { ThemeContext, createStyleSheet } from '../styles'; @@ -21,11 +21,20 @@ type Props = $ReadOnly<{| children: Node, |}>; +/** + * A floating box to contain autocomplete popups. + * + * Avoids the horizontal insets by adding appropriate margin. + */ export default function Popup(props: Props): Node { const themeContext = useContext(ThemeContext); return ( - + {props.children} - + ); } diff --git a/src/common/Screen.js b/src/common/Screen.js index f52b59cc3ee..ff1a83947a8 100644 --- a/src/common/Screen.js +++ b/src/common/Screen.js @@ -56,6 +56,8 @@ type Props = $ReadOnly<{| * Provides a nav bar, colors the status bar, can center the contents, etc. * The `children` are ultimately wrapped in a `ScrollView` from upstream. * + * Pads the bottom inset with a theme-appropriate background color. + * * @prop [centerContent] - Should the contents be centered. * @prop children - Components to render inside the screen. * @prop [keyboardShouldPersistTaps] - Passed through to ScrollView. diff --git a/src/common/SectionHeader.js b/src/common/SectionHeader.js index 3313fd1d117..c55f67cbc5f 100644 --- a/src/common/SectionHeader.js +++ b/src/common/SectionHeader.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React, { PureComponent } from 'react'; import type { Node, Context } from 'react'; -import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { ThemeData } from '../styles'; import { ThemeContext, createStyleSheet } from '../styles'; @@ -18,6 +18,11 @@ type Props = $ReadOnly<{| text: string, |}>; +/** + * (TODO: Is this meant to be reusable? How?) + * + * Pads the horizontal insets with its background. + */ export default class SectionHeader extends PureComponent { static contextType: Context = ThemeContext; context: ThemeData; @@ -25,9 +30,13 @@ export default class SectionHeader extends PureComponent { render(): Node { const { text } = this.props; return ( - + + ); } } diff --git a/src/common/SelectableOptionRow.js b/src/common/SelectableOptionRow.js index e29f4036276..318cab8d68f 100644 --- a/src/common/SelectableOptionRow.js +++ b/src/common/SelectableOptionRow.js @@ -2,6 +2,7 @@ import React from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import { RawLabel, Touchable } from '.'; import { BRAND_COLOR, createStyleSheet } from '../styles'; @@ -56,6 +57,8 @@ type Props = $ReadOnly<{| * it is either active or not. The event handler shouldn't do random * things that aren't related to that state, like navigating to a * different screen. + * + * Pads the horizontal insets with its background. */ export default function SelectableOptionRow( props: Props, @@ -64,13 +67,13 @@ export default function SelectableOptionRow( return ( onRequestSelectionChange(itemKey, !selected)}> - + {subtitle !== undefined && } {selected && } - + ); } diff --git a/src/common/ServerCompatBanner.js b/src/common/ServerCompatBanner.js index c13dcfeb7fc..69dfbcbe6c8 100644 --- a/src/common/ServerCompatBanner.js +++ b/src/common/ServerCompatBanner.js @@ -45,6 +45,8 @@ type Props = $ReadOnly<{||}>; /** * A "nag banner" saying the server version is unsupported, if so. + * + * Pads the horizontal insets with its background. */ // Made with somewhat careful attention to // https://material.io/components/banners. Please consult that before making diff --git a/src/common/SwitchRow.js b/src/common/SwitchRow.js index bc348f8a47c..0c8e47563d7 100644 --- a/src/common/SwitchRow.js +++ b/src/common/SwitchRow.js @@ -3,6 +3,7 @@ import React, { useContext } from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { SpecificIconType } from './Icons'; import Label from './Label'; @@ -25,6 +26,11 @@ const componentStyles = createStyleSheet({ /** * A row with a label and a switch component. + * + * Pads the horizontal insets with its background. (A parent component + * could probably do this instead, if desired. The choice to do it here is + * just in line with our other "row" components, like `SelectableOptionRow`, + * which do need to pad the insets.) */ export default function SwitchRow(props: Props): Node { const { label, value, onValueChange, style, Icon } = props; @@ -32,12 +38,16 @@ export default function SwitchRow(props: Props): Node { const themeContext = useContext(ThemeContext); return ( - + {!!Icon && } + ); } diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 7daf3f88f92..e10cc3aecf7 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -6,10 +6,9 @@ import type { DocumentPickerResponse } from 'react-native-document-picker'; import type { LayoutEvent } from 'react-native/Libraries/Types/CoreEventTypes'; // $FlowFixMe[untyped-import] import TextInputReset from 'react-native-text-input-reset'; -import { type EdgeInsets } from 'react-native-safe-area-context'; +import { SafeAreaView } from 'react-native-safe-area-context'; import { compose } from 'redux'; -import { withSafeAreaInsets } from '../react-native-safe-area-context'; import type { ThemeData } from '../styles'; import { ThemeContext } from '../styles'; import type { @@ -88,9 +87,6 @@ type Props = $ReadOnly<{| // From 'withGetText' _: GetText, - // from withSafeAreaInsets - insets: EdgeInsets, - // from `connect` dispatch: Dispatch, ...SelectorProps, @@ -134,6 +130,11 @@ const updateTextInput = (textInput, text) => { } }; +/** + * The compose box, for new messages or editing messages (PM or stream). + * + * Pads the bottom inset with its background. + */ class ComposeBoxInner extends PureComponent { static contextType = ThemeContext; context: ThemeData; @@ -478,7 +479,6 @@ class ComposeBoxInner extends PureComponent { narrow, allUsersById, editMessage, - insets, isAdmin, isAnnouncementOnly, isSubscribed, @@ -497,7 +497,6 @@ class ComposeBoxInner extends PureComponent { const placeholder = getComposeInputPlaceholder(narrow, ownUserId, allUsersById); const style = { - paddingBottom: insets.bottom, backgroundColor: 'hsla(0, 0%, 50%, 0.1)', }; @@ -518,7 +517,12 @@ class ComposeBoxInner extends PureComponent { onAutocomplete={this.handleMessageAutocomplete} /> - + { disabled={message.trim().length === 0 || this.state.numUploading > 0} onPress={editMessage === null ? this.handleSend : this.handleEdit} /> - + ); } @@ -586,7 +590,6 @@ const ComposeBox: ComponentType = compose( stream: getStreamInNarrow(state, props.narrow), videoChatProvider: getVideoChatProvider(state), })), - withSafeAreaInsets, )(withGetText(ComposeBoxInner)); export default ComposeBox; diff --git a/src/emoji/EmojiPickerScreen.js b/src/emoji/EmojiPickerScreen.js index 8cbe7eb9059..12c2e05eba3 100644 --- a/src/emoji/EmojiPickerScreen.js +++ b/src/emoji/EmojiPickerScreen.js @@ -32,6 +32,12 @@ type State = {| filter: string, |}; +/** + * A screen to choose an emoji reaction. + * + * Needs to occupy the horizontal insets because its descendents (the + * `EmojiRow`s) do. + */ class EmojiPickerScreen extends PureComponent { state = { filter: '', diff --git a/src/emoji/EmojiRow.js b/src/emoji/EmojiRow.js index 19f42edc989..7bf7b5e8d0f 100644 --- a/src/emoji/EmojiRow.js +++ b/src/emoji/EmojiRow.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React, { useCallback } from 'react'; import type { Node } from 'react'; -import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { EmojiType } from '../types'; import { createStyleSheet } from '../styles'; @@ -26,6 +26,11 @@ type Props = $ReadOnly<{| onPress: (name: string) => void, |}>; +/** + * A list item for an emoji, e.g., for autocomplete or the reaction picker. + * + * Pads the horizontal insets with its background. + */ export default function EmojiRow(props: Props): Node { const { code, name, type, onPress } = props; @@ -35,10 +40,10 @@ export default function EmojiRow(props: Props): Node { return ( - + - + ); } diff --git a/src/lightbox/LightboxFooter.js b/src/lightbox/LightboxFooter.js index 5da3f82c29b..8bfb08c9f80 100644 --- a/src/lightbox/LightboxFooter.js +++ b/src/lightbox/LightboxFooter.js @@ -33,6 +33,11 @@ type Props = $ReadOnly<{| onOptionsPress: () => void, |}>; +/** + * The lightbox's footer. + * + * Pads the right, bottom, and left insets with its background. + */ export default class LightboxFooter extends PureComponent { render(): Node { const { displayMessage, onOptionsPress, style } = this.props; diff --git a/src/lightbox/LightboxHeader.js b/src/lightbox/LightboxHeader.js index dd2f36cca98..c61e11c39ec 100644 --- a/src/lightbox/LightboxHeader.js +++ b/src/lightbox/LightboxHeader.js @@ -48,6 +48,8 @@ type Props = $ReadOnly<{| /** * Shows sender's name and date of photo being displayed. * + * Pads the top, right, and left insets with its background. + * * @prop [senderName] - The sender's full name. * @prop [avatarUrl] * @prop [timestamp] diff --git a/src/main/HomeScreen.js b/src/main/HomeScreen.js index 62068ae4004..72bcf7805f7 100644 --- a/src/main/HomeScreen.js +++ b/src/main/HomeScreen.js @@ -2,6 +2,7 @@ import React from 'react'; import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { RouteProp } from '../react-navigation'; import type { MainTabsNavigationProp } from './MainTabsScreen'; @@ -33,12 +34,18 @@ type Props = $ReadOnly<{| route: RouteProp<'home', void>, |}>; +/** + * The first tab in the main-tabs screen, showing unread counts. + * + * Needs to occupy the horizontal insets because its descendents (the + * unread conversation items) do. + */ export default function HomeScreen(props: Props) { const dispatch = useDispatch(); return ( - + { @@ -64,7 +71,7 @@ export default function HomeScreen(props: Props) { NavigationService.dispatch(navigateToSearch()); }} /> - + diff --git a/src/main/MainTabsScreen.js b/src/main/MainTabsScreen.js index 3c6723f7de5..86e9310e6da 100644 --- a/src/main/MainTabsScreen.js +++ b/src/main/MainTabsScreen.js @@ -45,6 +45,13 @@ type Props = $ReadOnly<{| route: RouteProp<'main-tabs', void>, |}>; +/** + * Wrapper and bottom tab navigator for the app's primary interface. + * + * Pads the top inset with a theme-appropriate background color. (This could + * be delegated to the individual tab screens, only if the screens promise + * not to disrupt a consistent look-and-feel.) + */ export default function MainTabsScreen(props: Props): Node { const { backgroundColor } = useContext(ThemeContext); diff --git a/src/nav/ChatNavBar.js b/src/nav/ChatNavBar.js index a2ae3e7f9cb..191a1bff4d4 100644 --- a/src/nav/ChatNavBar.js +++ b/src/nav/ChatNavBar.js @@ -22,6 +22,11 @@ type Props = $ReadOnly<{| editMessage: EditMessage | null, |}>; +/** + * `ChatScreen`'s top nav bar / app bar. + * + * Pads the top, right, and left insets with its background. + */ export default function ChatNavBar(props: Props): Node { const { narrow, editMessage } = props; const streamColor = useSelector(state => getStreamColorForNarrow(state, narrow)); diff --git a/src/nav/ModalNavBar.js b/src/nav/ModalNavBar.js index 72953db2ef7..73c7349c440 100644 --- a/src/nav/ModalNavBar.js +++ b/src/nav/ModalNavBar.js @@ -15,6 +15,11 @@ type Props = $ReadOnly<{| title: LocalizableText, |}>; +/** + * A flavor of top nav bar / app bar for Screen, with title and back button. + * + * Pads the top, right, and left insets with its background. + */ export default function ModalNavBar(props: Props): Node { const { canGoBack, title } = props; const { backgroundColor } = useContext(ThemeContext); diff --git a/src/nav/ModalSearchNavBar.js b/src/nav/ModalSearchNavBar.js index d0b1b9a56b1..06e7a463b67 100644 --- a/src/nav/ModalSearchNavBar.js +++ b/src/nav/ModalSearchNavBar.js @@ -13,6 +13,11 @@ type Props = $ReadOnly<{| canGoBack?: boolean, |}>; +/** + * A flavor of top nav / app bar for Screen with search bar and back button. + * + * Pads the top, right, and left insets with its background. + */ export default function ModalSearchNavBar(props: Props): Node { const { autoFocus, searchBarOnChange, canGoBack = true } = props; const { backgroundColor } = useContext(ThemeContext); diff --git a/src/pm-conversations/GroupPmConversationItem.js b/src/pm-conversations/GroupPmConversationItem.js index 3327454dd4d..d8235b35fee 100644 --- a/src/pm-conversations/GroupPmConversationItem.js +++ b/src/pm-conversations/GroupPmConversationItem.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React, { useCallback, useContext } from 'react'; import type { Node } from 'react'; -import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import { useSelector } from '../react-redux'; import type { UserOrBot } from '../types'; @@ -26,7 +26,9 @@ type Props = $ReadOnly<{| /** * A list item describing one group PM conversation. - * */ + * + * Pads the horizontal insets with its background. + */ export default function GroupPmConversationItem>( props: Props, ): Node { @@ -44,7 +46,7 @@ export default function GroupPmConversationItem>( return ( - + >( text={names.join(', ')} /> - + ); } diff --git a/src/pm-conversations/PmConversationList.js b/src/pm-conversations/PmConversationList.js index 4976d7c50d8..1fe549b8a1b 100644 --- a/src/pm-conversations/PmConversationList.js +++ b/src/pm-conversations/PmConversationList.js @@ -26,7 +26,10 @@ type Props = $ReadOnly<{| /** * A list describing all PM conversations. - * */ + * + * Needs to occupy the horizontal insets because its descendents (the PM + * conversation items) do. + */ export default function PmConversationList(props: Props): Node { const dispatch = useDispatch(); diff --git a/src/pm-conversations/PmConversationsScreen.js b/src/pm-conversations/PmConversationsScreen.js index f5fcb1d3b73..a2dd3972e50 100644 --- a/src/pm-conversations/PmConversationsScreen.js +++ b/src/pm-conversations/PmConversationsScreen.js @@ -3,6 +3,7 @@ import React, { useContext } from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; import type { RouteProp } from '../react-navigation'; import type { MainTabsNavigationProp } from '../main/MainTabsScreen'; @@ -41,14 +42,17 @@ type Props = $ReadOnly<{| /** * The "PMs" page in the main tabs navigation. - * */ + * + * Needs to occupy the horizontal insets because its descendents (the PM + * conversation items) do. + */ export default function PmConversationsScreen(props: Props): Node { const conversations = useSelector(getRecentConversations); const context = useContext(ThemeContext); return ( - + NavigationService.dispatch(navigateToUsersScreen())); }} /> - + {conversations.length === 0 ? (