Skip to content

Commit

Permalink
StreamSettingsScreen: s/Private/Privacy/; use radio buttons, not a sw…
Browse files Browse the repository at this point in the history
…itch

For zulip#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.
  • Loading branch information
chrisbobbe committed Apr 28, 2022
1 parent 02b603c commit fc9cba4
Show file tree
Hide file tree
Showing 8 changed files with 282 additions and 18 deletions.
3 changes: 3 additions & 0 deletions src/common/Icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
154 changes: 154 additions & 0 deletions src/common/InputRowRadioButtons.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/* @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 { BRAND_COLOR, createStyleSheet } from '../styles';

import Touchable from './Touchable';
import ZulipText from './ZulipText';
import { IconRight } from './Icons';
import type { AppNavigationProp } from '../nav/AppNavigator';

type Item<TKey> = $ReadOnly<{|
key: TKey,
title: string,
subtitle?: string,
|}>;

type Props<TItemKey> = $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: string,

/** What the setting is about, in a short sentence or two. */
description?: string,

/** 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<Item<TItemKey>>,
*/
items: $ReadOnlyArray<Item<TItemKey>>,

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<TItemKey: string>(props: Props<TItemKey>): Node {
const { navigation, label, description, valueKey, items, onValueChange } = props;

const screenKey = useRef(
`selectable-options-${Math.floor(Math.random() * Number.MAX_SAFE_INTEGER).toString(36)}`,
).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(() => {
// TODO: Can use .push instead of .navigate? See if Flow types are wrong
// in saying we can't pass `key` when using .push.
navigation.navigate({
name: 'selectable-options',
key: screenKey,
params: screenParams,
});
}, [navigation, screenKey, screenParams]);

// Live-update the selectable-options screen.
useEffect(() => {
navigation.dispatch(
state =>
state.routes.find(route => route.key === screenKey)
? { ...CommonActions.setParams(screenParams), source: screenKey }
: CommonActions.reset(state), // no-op
);
}, [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 (
<Touchable onPress={handleRowPressed}>
<View style={styles.wrapper}>
<View style={styles.textWrapper}>
<ZulipText text={label} />
<ZulipText text={selectedItem.title} style={styles.valueTitle} />
</View>
<View style={styles.iconRightWrapper}>
<IconRight size={kRightArrowIconSize} color={BRAND_COLOR} />
</View>
</View>
</Touchable>
);
}
101 changes: 101 additions & 0 deletions src/common/SelectableOptionsScreen.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/* @flow strict-local */

import React, { useMemo } from 'react';
import type { Node } from 'react';
import { LogBox, View } from 'react-native';

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<TKey> = $ReadOnly<{|
key: TKey,
title: string,
subtitle?: string,
selected: boolean,
|}>;

type Props<TItemKey> = $ReadOnly<{|
navigation: AppNavigationProp<'selectable-options'>,
route: RouteProp<
'selectable-options',
{|
title: string,

/**
* An optional explanation, to be shown before the items.
*/
description?: string,

items: $ReadOnlyArray<Item<TItemKey>>,

// 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<TItemKey: string>(props: Props<TItemKey>): Node {
const { route } = props;
const { title, description, items, onRequestSelectionChange } = route.params;

const styles = useMemo(
() =>
createStyleSheet({
descriptionWrapper: { padding: 16 },
}),
[],
);

return (
<Screen title={title} scrollEnabled>
{description != null && (
<View style={styles.descriptionWrapper}>
<ZulipTextIntl text={description} />
</View>
)}
{items.map(item => (
<SelectableOptionRow
key={item.key}
itemKey={item.key}
title={item.title}
subtitle={item.subtitle}
selected={item.selected}
onRequestSelectionChange={onRequestSelectionChange}
/>
))}
</Screen>
);
}
3 changes: 3 additions & 0 deletions src/nav/AppNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {|
Expand Down Expand Up @@ -78,6 +79,7 @@ export type AppNavigatorParamList = {|
'user-status': RouteParamsOf<typeof UserStatusScreen>,
sharing: RouteParamsOf<typeof SharingScreen>,
settings: RouteParamsOf<typeof SettingsScreen>,
'selectable-options': RouteParamsOf<typeof SelectableOptionsScreen>,
|};

export type AppNavigationProp<
Expand Down Expand Up @@ -162,6 +164,7 @@ export default function AppNavigator(props: Props): Node {
initialParams={initialRouteName === 'realm-input' ? initialRouteParams : undefined}
/>
<Stack.Screen name="sharing" component={SharingScreen} />
<Stack.Screen name="selectable-options" component={SelectableOptionsScreen} />
</Stack.Navigator>
);
}
2 changes: 2 additions & 0 deletions src/streams/CreateStreamScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Props = $ReadOnly<{|
|}>;

export default function CreateStreamScreen(props: Props): Node {
const { navigation } = props;
const auth = useSelector(getAuth);

const handleComplete = useCallback(
Expand All @@ -31,6 +32,7 @@ export default function CreateStreamScreen(props: Props): Node {
return (
<Screen title="Create new stream" padding>
<EditStreamCard
navigation={navigation}
isNewStream
initialValues={{ name: '', description: '', privacy: 'public' }}
onComplete={handleComplete}
Expand Down
34 changes: 16 additions & 18 deletions src/streams/EditStreamCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,20 @@ import React, { useState, useCallback } from 'react';
import type { Node } from 'react';
import { View } from 'react-native';

import type { AppNavigationProp } from '../nav/AppNavigator';
import Input from '../common/Input';
import InputRowRadioButtons from '../common/InputRowRadioButtons';
import ZulipTextIntl from '../common/ZulipTextIntl';
import SwitchRow from '../common/SwitchRow';
import ZulipButton from '../common/ZulipButton';
import styles, { createStyleSheet } from '../styles';
import { IconPrivate } from '../common/Icons';
import styles from '../styles';

/* eslint-disable no-shadow */

const componentStyles = createStyleSheet({
switchRow: {
paddingLeft: 8,
paddingRight: 8,
},
});

type Privacy = 'public' | 'private';

type Props = $ReadOnly<{|
navigation: AppNavigationProp<'edit-stream' | 'create-stream'>,

isNewStream: boolean,
initialValues: {|
name: string,
Expand All @@ -32,7 +27,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<string>(props.initialValues.name);
const [description, setDescription] = useState<string>(props.initialValues.description);
Expand All @@ -50,8 +45,8 @@ export default function EditStreamCard(props: Props): Node {
setDescription(description);
}, []);

const handlePrivacyChange = useCallback(isPrivate => {
setPrivacy(isPrivate ? 'private' : 'public');
const handlePrivacyChange = useCallback((privacy: Privacy) => {
setPrivacy(privacy);
}, []);

return (
Expand All @@ -71,11 +66,14 @@ export default function EditStreamCard(props: Props): Node {
defaultValue={initialValues.description}
onChangeText={handleDescriptionChange}
/>
<SwitchRow
style={componentStyles.switchRow}
Icon={IconPrivate}
label="Private"
value={privacy === 'private'}
<InputRowRadioButtons
navigation={navigation}
label="Privacy"
items={[
{ key: 'public', title: 'Public' },
{ key: 'private', title: 'Private' },
]}
valueKey={privacy}
onValueChange={handlePrivacyChange}
/>
<ZulipButton
Expand Down
2 changes: 2 additions & 0 deletions src/streams/EditStreamScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type Props = $ReadOnly<{|
|}>;

export default function EditStreamScreen(props: Props): Node {
const { navigation } = props;
const dispatch = useDispatch();
const stream = useSelector(state => getStreamForId(state, props.route.params.streamId));

Expand All @@ -37,6 +38,7 @@ export default function EditStreamScreen(props: Props): Node {
return (
<Screen title="Edit stream" padding>
<EditStreamCard
navigation={navigation}
isNewStream={false}
initialValues={{
name: stream.name,
Expand Down
Loading

0 comments on commit fc9cba4

Please sign in to comment.