Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

layout: Fix warts in NestedNavRow and SwitchRow #5446

Merged
merged 10 commits into from
Aug 31, 2022
47 changes: 38 additions & 9 deletions src/common/NestedNavRow.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* @flow strict-local */
import React, { useContext } from 'react';
import React, { useContext, useMemo } from 'react';
import type { Node } from 'react';
import { View } from 'react-native';

Expand All @@ -8,13 +8,17 @@ import ZulipTextIntl from './ZulipTextIntl';
import Touchable from './Touchable';
import { IconRight } from './Icons';
import type { SpecificIconType } from './Icons';
import styles, { ThemeContext } from '../styles';
import globalStyles, { ThemeContext, createStyleSheet } from '../styles';

type Props = $ReadOnly<{|
Icon?: SpecificIconType,
label: LocalizableReactText,

// Use this to navigate to a "nested" screen.
// TODO: Should we make this unconfigurable? Should we have two reusable
// components, with and without this?
labelBoldUppercase?: true,

/** Use this to navigate to a "nested" screen. */
onPress: () => void,
|}>;

Expand All @@ -25,17 +29,42 @@ type Props = $ReadOnly<{|
* selectable option row instead, use `SelectableOptionRow`.
*/
export default function NestedNavRow(props: Props): Node {
const { label, onPress, Icon } = props;
const { label, labelBoldUppercase, onPress, Icon } = props;

const themeContext = useContext(ThemeContext);

const styles = useMemo(
() =>
createStyleSheet({
container: {
// Minimum touch target height (and width):
// https://material.io/design/usability/accessibility.html#layout-and-typography
minHeight: 48,
},
iconFromProps: {
textAlign: 'center',
marginRight: 8,
color: themeContext.color,
},
label: {
...(labelBoldUppercase ? { textTransform: 'uppercase', fontWeight: '500' } : undefined),
},
iconRightFacingArrow: {
textAlign: 'center',
marginLeft: 8,
color: themeContext.color,
},
}),
[themeContext, labelBoldUppercase],
);

return (
<Touchable onPress={onPress}>
<View style={styles.listItem}>
{!!Icon && <Icon size={24} style={[styles.settingsIcon, { color: themeContext.color }]} />}
<ZulipTextIntl text={label} />
<View style={styles.rightItem}>
<IconRight size={24} style={[styles.settingsIcon, { color: themeContext.color }]} />
<View style={[styles.container, globalStyles.listItem]}>
{!!Icon && <Icon size={24} style={styles.iconFromProps} />}
<ZulipTextIntl style={styles.label} text={label} />
<View style={globalStyles.rightItem}>
<IconRight size={24} style={styles.iconRightFacingArrow} />
</View>
</View>
</Touchable>
Expand Down
17 changes: 11 additions & 6 deletions src/common/SwitchRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
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 type { SpecificIconType } from './Icons';
import ZulipTextIntl from './ZulipTextIntl';
Expand All @@ -13,27 +12,33 @@ type Props = $ReadOnly<{|
Icon?: SpecificIconType,
label: string,
value: boolean,
style?: ViewStyleProp,
onValueChange: (newValue: boolean) => void,
|}>;

const componentStyles = createStyleSheet({
container: {
height: 56,
// For uniformity with other rows this might share a screen with, e.g.,
// NestedNavRow on the settings screen. See height-related attributes on
// those rows.
minHeight: 48,
},
icon: {
textAlign: 'center',
marginRight: 8,
},
});

/**
* A row with a label and a switch component.
*/
export default function SwitchRow(props: Props): Node {
const { label, value, onValueChange, style, Icon } = props;
const { label, value, onValueChange, Icon } = props;

const themeContext = useContext(ThemeContext);

return (
<View style={[componentStyles.container, styles.listItem, style]}>
{!!Icon && <Icon size={24} style={[styles.settingsIcon, { color: themeContext.color }]} />}
<View style={[componentStyles.container, styles.listItem]}>
{!!Icon && <Icon size={24} style={[componentStyles.icon, { color: themeContext.color }]} />}
<ZulipTextIntl text={label} style={styles.flexed} />
<View style={styles.rightItem}>
<ZulipSwitch value={value} onValueChange={onValueChange} />
Expand Down
31 changes: 4 additions & 27 deletions src/streams/SubscriptionsScreen.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/* @flow strict-local */

import React, { useCallback, useMemo, useContext } from 'react';
import React, { useCallback, useMemo } from 'react';
import type { Node } from 'react';
import { View, SectionList } from 'react-native';

import { useNavigation } from '../react-navigation';
import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import type { Subscription } from '../types';
import appStyles, { createStyleSheet, ThemeContext } from '../styles';
import { createStyleSheet } from '../styles';
import { useDispatch, useSelector } from '../react-redux';
import LoadingBanner from '../common/LoadingBanner';
import SectionSeparatorBetween from '../common/SectionSeparatorBetween';
Expand All @@ -20,9 +20,7 @@ import { doNarrow } from '../actions';
import { caseInsensitiveCompareFunc } from '../utils/misc';
import StreamItem from './StreamItem';
import ModalNavBar from '../nav/ModalNavBar';
import ZulipTextIntl from '../common/ZulipTextIntl';
import Touchable from '../common/Touchable';
import { IconRight } from '../common/Icons';
import NestedNavRow from '../common/NestedNavRow';

const styles = createStyleSheet({
container: {
Expand All @@ -33,19 +31,6 @@ const styles = createStyleSheet({
flex: 1,
flexDirection: 'column',
},
rightItem: {
marginLeft: 'auto',
},
rightIcon: {
marginLeft: 'auto',
},
allStreamsButton: {
paddingRight: 12,
},
streamsText: {
textTransform: 'uppercase',
fontWeight: '500',
},
});

type Props = $ReadOnly<{|
Expand All @@ -57,19 +42,11 @@ type FooterProps = $ReadOnly<{||}>;

function AllStreamsButton(props: FooterProps): Node {
const navigation = useNavigation();
const themeContext = useContext(ThemeContext);
const handlePressAllScreens = useCallback(() => {
navigation.push('all-streams');
}, [navigation]);

return (
<Touchable onPress={handlePressAllScreens}>
<View style={[appStyles.listItem, styles.allStreamsButton]}>
<ZulipTextIntl style={styles.streamsText} text="All streams" />
<IconRight size={24} style={[styles.rightIcon, { color: themeContext.color }]} />
</View>
</Touchable>
);
return <NestedNavRow label="All streams" labelBoldUppercase onPress={handlePressAllScreens} />;
}

export default function SubscriptionsScreen(props: Props): Node {
Expand Down
6 changes: 0 additions & 6 deletions src/styles/miscStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ export const statics = {
flexDirection: 'row',
alignItems: 'center',
},
settingsIcon: {
margin: 8,
textAlign: 'center',
marginLeft: 8,
marginRight: 16,
},
listItem: {
flexDirection: 'row',
alignItems: 'center',
Expand Down