From 5ceec85aed19623eeae2a1f0f0c32e00cdab7316 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 25 Apr 2022 17:07:27 -0700 Subject: [PATCH] StreamSettingsScreen: s/Private/Privacy/; use radio buttons, not a switch For #5250, we'll need to let the user choose one among more than two options. That'll call for a radio-button-style input, rather than a switch. So, make a new component InputRowRadioButtons, and use it for an input labeled "Privacy", replacing the SwitchRow input labeled "Private". And, to support that component, write and wire up another new component, SelectableOptionsScreen. --- src/common/Icons.js | 3 + src/common/InputRowRadioButtons.js | 160 ++++++++++++++++++++++++++ src/common/SelectableOptionsScreen.js | 102 ++++++++++++++++ src/nav/AppNavigator.js | 3 + src/streams/CreateStreamScreen.js | 2 + src/streams/EditStreamCard.js | 42 ++++--- src/streams/EditStreamScreen.js | 2 + static/translations/messages_en.json | 1 + 8 files changed, 296 insertions(+), 19 deletions(-) create mode 100644 src/common/InputRowRadioButtons.js create mode 100644 src/common/SelectableOptionsScreen.js diff --git a/src/common/Icons.js b/src/common/Icons.js index b768bf44ff3..dca14090b1b 100644 --- a/src/common/Icons.js +++ b/src/common/Icons.js @@ -85,7 +85,10 @@ export const IconDiagnostics: SpecificIconType = makeIcon(Feather, 'activity'); export const IconNotifications: SpecificIconType = makeIcon(Feather, 'bell'); export const IconLanguage: SpecificIconType = makeIcon(Feather, 'globe'); export const IconSettings: SpecificIconType = makeIcon(Feather, 'settings'); + +// InputRowRadioButtons depends on this being square. export const IconRight: SpecificIconType = makeIcon(Feather, 'chevron-right'); + export const IconPlusCircle: SpecificIconType = makeIcon(Feather, 'plus-circle'); export const IconLeft: SpecificIconType = makeIcon(Feather, 'chevron-left'); export const IconPeople: SpecificIconType = makeIcon(Feather, 'users'); diff --git a/src/common/InputRowRadioButtons.js b/src/common/InputRowRadioButtons.js new file mode 100644 index 00000000000..38214ee9aca --- /dev/null +++ b/src/common/InputRowRadioButtons.js @@ -0,0 +1,160 @@ +/* @flow strict-local */ +import React, { useCallback, useRef, useMemo, useEffect } from 'react'; +import type { Node } from 'react'; +import { View } from 'react-native'; +import invariant from 'invariant'; +import { CommonActions } from '@react-navigation/native'; + +import type { LocalizableText } from '../types'; +import { randString } from '../utils/misc'; +import { BRAND_COLOR, createStyleSheet } from '../styles'; +import Touchable from './Touchable'; +import ZulipTextIntl from './ZulipTextIntl'; +import { IconRight } from './Icons'; +import type { AppNavigationProp } from '../nav/AppNavigator'; + +type Item = $ReadOnly<{| + key: TKey, + title: LocalizableText, + subtitle?: LocalizableText, +|}>; + +type Props = $ReadOnly<{| + /** + * Component must be under the stack nav with the selectable-options screen. + * + * Pass this down from props or `useNavigation`. + */ + navigation: AppNavigationProp<>, + + /** What the setting is about, e.g., "Theme". */ + label: LocalizableText, + + /** What the setting is about, in a short sentence or two. */ + description?: LocalizableText, + + /** The current value of the input. */ + valueKey: TItemKey, + + /** + * The available options to choose from, with unique `key`s, one of which + * must be `valueKey`. + */ + items: $ReadOnlyArray>, + + onValueChange: (newValue: TItemKey) => void, +|}>; + +/** + * A form-input row for the user to make a choice, radio-button style. + * + * Shows the current value (the selected item), represented as the item's + * `.title`. When tapped, opens the selectable-options screen, where the + * user can change the selection or read more about each selection. + */ +// This has the navigate-to-nested-screen semantics of NestedNavRow, +// represented by IconRight. NestedNavRow would probably be the wrong +// abstraction, though, because it isn't an imput component; it doesn't have +// a value to display. +export default function InputRowRadioButtons(props: Props): Node { + const { navigation, label, description, valueKey, items, onValueChange } = props; + + const screenKey = useRef(`selectable-options-${randString()}`).current; + + const selectedItem = items.find(c => c.key === valueKey); + invariant(selectedItem != null, 'InputRowRadioButtons: exactly one choice must be selected'); + + const screenParams = useMemo( + () => ({ + title: label, + description, + items: items.map(({ key, title, subtitle }) => ({ + key, + title, + subtitle, + selected: key === valueKey, + })), + onRequestSelectionChange: (itemKey, requestedValue) => { + if (!requestedValue || itemKey === valueKey) { + // Can't unselect a radio button. + return; + } + navigation.goBack(); + onValueChange(itemKey); + }, + }), + [navigation, label, description, items, valueKey, onValueChange], + ); + + const handleRowPressed = useCallback(() => { + // Normally we'd use `.push`, to avoid `.navigate`'s funky + // rewind-history behavior. But `.push` doesn't accept a custom key, so + // we use `.navigate`. This is fine because the funky rewind-history + // behavior can't happen here; see + // https://github.com/zulip/zulip-mobile/pull/5369#discussion_r868799934 + navigation.navigate({ + name: 'selectable-options', + key: screenKey, + params: screenParams, + }); + }, [navigation, screenKey, screenParams]); + + // Live-update the selectable-options screen. + useEffect(() => { + navigation.dispatch(state => + /* eslint-disable operator-linebreak */ + state.routes.find(route => route.key === screenKey) + ? { ...CommonActions.setParams(screenParams), source: screenKey } + : // A screen with key `screenKey` isn't currently mounted on the + // navigator. Don't refer to such a screen; that would be an + // error. + CommonActions.reset(state), + ); + }, [navigation, screenParams, screenKey]); + + // The desired height for IconRight, which we'll pass for its `size` prop. + // It'll also be its width. + const kRightArrowIconSize = 24; + + const styles = useMemo( + () => + createStyleSheet({ + wrapper: { + flexDirection: 'row', + alignItems: 'center', + paddingVertical: 8, + paddingHorizontal: 16, + }, + textWrapper: { + flex: 1, + flexDirection: 'column', + }, + valueTitle: { + fontWeight: '300', + fontSize: 13, + }, + iconRightWrapper: { + height: kRightArrowIconSize, + + // IconRight is a square, so width equals height and this is the + // right amount of width to reserve. + width: kRightArrowIconSize, + }, + }), + [], + ); + + return ( + + + + + + + + + + + + ); +} diff --git a/src/common/SelectableOptionsScreen.js b/src/common/SelectableOptionsScreen.js new file mode 100644 index 00000000000..77c45921aca --- /dev/null +++ b/src/common/SelectableOptionsScreen.js @@ -0,0 +1,102 @@ +/* @flow strict-local */ + +import React, { useMemo } from 'react'; +import type { Node } from 'react'; +import { LogBox, View } from 'react-native'; + +import type { LocalizableText } from '../types'; +import type { RouteProp } from '../react-navigation'; +import type { AppNavigationProp } from '../nav/AppNavigator'; +import Screen from './Screen'; +import SelectableOptionRow from './SelectableOptionRow'; +import ZulipTextIntl from './ZulipTextIntl'; +import { createStyleSheet } from '../styles'; + +type Item = $ReadOnly<{| + key: TKey, + title: LocalizableText, + subtitle?: LocalizableText, + selected: boolean, +|}>; + +type Props = $ReadOnly<{| + navigation: AppNavigationProp<'selectable-options'>, + route: RouteProp< + 'selectable-options', + {| + title: LocalizableText, + + /** + * An optional explanation, to be shown before the items. + */ + description?: LocalizableText, + + items: $ReadOnlyArray>, + + // This param is a function, so React Nav is right to point out that + // it isn't serializable. But this is fine as long as we don't try to + // persist navigation state for this screen or set up deep linking to + // it, hence the LogBox suppression below. + // + // React Navigation doesn't offer a more sensible way to have us pass + // the selection to the calling screen. …We could store the selection + // as a route param on the calling screen, or in Redux. But from this + // screen's perspective, that's basically just setting a global + // variable. Better to offer this explicit, side-effect-free way for + // the data to flow where it should, when it should. + onRequestSelectionChange: (itemKey: TItemKey, requestedValue: boolean) => void, + |}, + >, +|}>; + +// React Navigation would give us a console warning about non-serializable +// route params. For more about the warning, see +// https://reactnavigation.org/docs/5.x/troubleshooting/#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state +// See comment on this param, above. +LogBox.ignoreLogs([/selectable-options > params\.onRequestSelectionChange \(Function\)/]); + +/** + * A screen for selecting items in a list, checkbox or radio-button style. + * + * The items and selection state are controlled by the route params. Callers + * should declare a unique key for their own use of this route, as follows, + * so that instances can't step on each other: + * + * navigation.push({ name: 'selectable-options', key: 'foo', params: { … } }) + * navigation.dispatch({ ...CommonActions.setParams({ … }), source: 'foo' }) + */ +// If we need separate components dedicated to checkboxes and radio buttons, +// we can split this. Currently it's up to the caller to enforce the +// radio-button invariant (exactly one item selected) if they want to. +export default function SelectableOptionsScreen(props: Props): Node { + const { route } = props; + const { title, description, items, onRequestSelectionChange } = route.params; + + const styles = useMemo( + () => + createStyleSheet({ + descriptionWrapper: { padding: 16 }, + }), + [], + ); + + return ( + + {description != null && ( + + + + )} + {items.map(item => ( + + ))} + + ); +} diff --git a/src/nav/AppNavigator.js b/src/nav/AppNavigator.js index 5f2a0279681..36f06f058a8 100644 --- a/src/nav/AppNavigator.js +++ b/src/nav/AppNavigator.js @@ -44,6 +44,7 @@ import LegalScreen from '../settings/LegalScreen'; import SettingsScreen from '../settings/SettingsScreen'; import UserStatusScreen from '../user-statuses/UserStatusScreen'; import SharingScreen from '../sharing/SharingScreen'; +import SelectableOptionsScreen from '../common/SelectableOptionsScreen'; import { useHaveServerDataGate } from '../withHaveServerDataGate'; export type AppNavigatorParamList = {| @@ -78,6 +79,7 @@ export type AppNavigatorParamList = {| 'user-status': RouteParamsOf, sharing: RouteParamsOf, settings: RouteParamsOf, + 'selectable-options': RouteParamsOf, |}; export type AppNavigationProp< @@ -162,6 +164,7 @@ export default function AppNavigator(props: Props): Node { initialParams={initialRouteName === 'realm-input' ? initialRouteParams : undefined} /> + ); } diff --git a/src/streams/CreateStreamScreen.js b/src/streams/CreateStreamScreen.js index 94c764f6566..3357a18b2e8 100644 --- a/src/streams/CreateStreamScreen.js +++ b/src/streams/CreateStreamScreen.js @@ -23,6 +23,7 @@ type Props = $ReadOnly<{| export default function CreateStreamScreen(props: Props): Node { const _ = useContext(TranslationContext); + const { navigation } = props; const auth = useSelector(getAuth); const streamsByName = useSelector(getStreamsByName); @@ -62,6 +63,7 @@ export default function CreateStreamScreen(props: Props): Node { return ( , + isNewStream: boolean, initialValues: {| name: string, @@ -32,7 +28,7 @@ type Props = $ReadOnly<{| |}>; export default function EditStreamCard(props: Props): Node { - const { onComplete, initialValues, isNewStream } = props; + const { navigation, onComplete, initialValues, isNewStream } = props; const [name, setName] = useState(props.initialValues.name); const [description, setDescription] = useState(props.initialValues.description); @@ -50,10 +46,18 @@ export default function EditStreamCard(props: Props): Node { setDescription(description); }, []); - const handlePrivacyChange = useCallback(isPrivate => { - setPrivacy(isPrivate ? 'private' : 'public'); + const handlePrivacyChange = useCallback((privacy: Privacy) => { + setPrivacy(privacy); }, []); + const privacyOptions = useMemo<$ReadOnlyArray<{| key: Privacy, title: LocalizableText |}>>( + () => [ + { key: 'public', title: 'Public' }, + { key: 'private', title: 'Private' }, + ], + [], + ); + return ( @@ -71,11 +75,11 @@ export default function EditStreamCard(props: Props): Node { defaultValue={initialValues.description} onChangeText={handleDescriptionChange} /> - ; export default function EditStreamScreen(props: Props): Node { + const { navigation } = props; const dispatch = useDispatch(); const stream = useSelector(state => getStreamForId(state, props.route.params.streamId)); @@ -37,6 +38,7 @@ export default function EditStreamScreen(props: Props): Node { return (