Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
fb7a49e
ZulipStatusBar [nfc]: Un-nest `StatusBar` from `View`.
chrisbobbe Jan 25, 2021
fc08d86
Screen [nfc]: Grab `orientation` from Redux.
chrisbobbe Jan 25, 2021
6956fc0
ZulipStatusBar [nfc]: Move sized `View` out to callers.
chrisbobbe Jan 25, 2021
bac28e4
ZulipStatusBar [nfc]: Settle into a recent change (1/x).
chrisbobbe Jan 25, 2021
59a2d4c
ZulipStatusBar [nfc]: Settle into a recent change (2/x).
chrisbobbe Jan 25, 2021
c1ab47e
ZulipStatusBar [nfc]: Settle into a recent change (3/x).
chrisbobbe Jan 25, 2021
919c71d
LightboxScreen [nfc]: Remove an invisible, zero-height `View`.
chrisbobbe Jan 26, 2021
4f1fcff
safe-area: Remove an unnecessary condition on `orientation`.
chrisbobbe Jan 26, 2021
e7520ac
ChatScreen [nfc]: Reorder `View` with `ZulipStatusBar`.
chrisbobbe Jan 26, 2021
f06bbad
ChatScreen [nfc]: Move safe-area-handling `View` into `ChatNavBar`.
chrisbobbe Jan 26, 2021
84c7088
ChatNavBar [nfc]: Better integrate safe-area handling (1/x).
chrisbobbe Jan 26, 2021
2f02651
ChatNavBar [nfc]: Better integrate safe-area handling (2/x).
chrisbobbe Jan 26, 2021
86b09d3
ChatNavBar: Better integrate safe-area handling (3/x).
chrisbobbe Jan 26, 2021
909a3f7
ChatNavBar: Handle left and right insets too.
chrisbobbe Jan 26, 2021
b89634a
SettingsScreen: Remove `ModalNavBar`.
chrisbobbe Jan 27, 2021
08c1904
Screen [nfc]: Reorder `View` with `ZulipStatusBar`.
chrisbobbe Jan 27, 2021
746a9f6
Screen [nfc]: Move safe-area-handling `View` into `Modal(Search)NavBar`.
chrisbobbe Jan 27, 2021
635b13a
ModalNavBar: Remove explicit height.
chrisbobbe Jan 30, 2021
3f5bcf3
ModalNavBar: Better integrate safe-area handling (1/x).
chrisbobbe Jan 27, 2021
c5f340d
ModalNavBar: Better integrate safe-area handling (2/x).
chrisbobbe Jan 27, 2021
fd81cc7
ModalNavBar: Handle left and right insets too.
chrisbobbe Jan 27, 2021
a9ab3fc
ModalSearchNavBar: Remove explicit height.
chrisbobbe Jan 30, 2021
93d9efa
ModalSearchNavBar: Better integrate safe-area handling (1/x).
chrisbobbe Jan 27, 2021
d1fd30d
ModalSearchNavBar: Better integrate safe-area handling (2/x).
chrisbobbe Jan 27, 2021
deb3015
ModalSearchNavBar: Handle left and right insets too.
chrisbobbe Jan 27, 2021
26f6f23
ChatNavBar [nfc]: Pass `undefined` instead of 'transparent'.
chrisbobbe Jan 30, 2021
5abde56
ChatNavBar [nfc]: Use a variable instead of 'transparent'.
chrisbobbe Jan 30, 2021
a9c1b44
nav [nfc]: Remove DEFAULT_TITLE_BACKGROUND_COLOR.
chrisbobbe Jan 30, 2021
42396b6
nav [nfc]: Simplify a few conditionals.
chrisbobbe Jan 30, 2021
d16cdfa
ZulipStatusBar [nfc]: Remove default prop with value `undefined`.
chrisbobbe Feb 1, 2021
dada934
titleSelectors [nfc]: Give `getTitleBackgroundColor` a better name.
chrisbobbe Jan 30, 2021
6782cfd
titleSelectors [nfc]: Make an early return earlier.
chrisbobbe Jan 30, 2021
2288807
nav [nfc]: Improve a few variable names.
chrisbobbe Jan 30, 2021
0a218e0
titleSelectors [nfc]: Delete; move only selector to subscriptionSelec…
chrisbobbe Feb 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/chat/ChatScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { canSendToNarrow } from '../utils/narrow';
import { getLoading, getSession } from '../directSelectors';
import { getFetchingForNarrow } from './fetchingSelectors';
import { getShownMessagesForNarrow, isNarrowValid as getIsNarrowValid } from './narrowsSelectors';
import { getTitleBackgroundColor } from '../title/titleSelectors';
import { getStreamColorForNarrow } from '../subscriptions/subscriptionSelectors';

type Props = $ReadOnly<{|
navigation: AppNavigationProp<'chat'>,
Expand Down Expand Up @@ -114,13 +114,13 @@ export default function ChatScreen(props: Props) {
const sayNoMessages = haveNoMessages && !isFetching;
const showComposeBox = canSendToNarrow(narrow) && !showMessagePlaceholders;

const titleBackgroundColor = useSelector(state => getTitleBackgroundColor(state, narrow));
const streamColor = useSelector(state => getStreamColorForNarrow(state, narrow));

return (
<ActionSheetProvider>
<View style={[componentStyles.screen, { backgroundColor }]}>
<KeyboardAvoider style={styles.flexed} behavior="padding">
<ZulipStatusBar backgroundColor={titleBackgroundColor} />
<ZulipStatusBar backgroundColor={streamColor} />
<ChatNavBar narrow={narrow} editMessage={editMessage} />
<OfflineNotice />
<UnreadNotice narrow={narrow} />
Expand Down
52 changes: 19 additions & 33 deletions src/common/ZulipStatusBar.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,18 @@
/* @flow strict-local */

import React, { PureComponent } from 'react';
import { Platform, StatusBar, View } from 'react-native';
import { Platform, StatusBar } from 'react-native';
import Color from 'color';
import { compose } from 'redux';
import { EdgeInsets } from 'react-native-safe-area-context';

import { withSafeAreaInsets } from '../react-native-safe-area-context';
import type { Orientation, ThemeName, Dispatch } from '../types';
import { connect } from '../react-redux';
import { DEFAULT_TITLE_BACKGROUND_COLOR } from '../title/titleSelectors';
import { foregroundColorFromBackground } from '../utils/color';
import { getSession, getSettings } from '../selectors';

type BarStyle = $PropertyType<React$ElementConfig<typeof StatusBar>, 'barStyle'>;

export const getStatusBarColor = (backgroundColor: string, theme: ThemeName): string =>
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
? theme === 'night'
? 'hsl(212, 28%, 18%)'
: 'white'
: backgroundColor;
export const getStatusBarColor = (backgroundColor: string | void, theme: ThemeName): string =>
backgroundColor ?? (theme === 'night' ? 'hsl(212, 28%, 18%)' : 'white');

export const getStatusBarStyle = (statusBarColor: string): BarStyle =>
foregroundColorFromBackground(statusBarColor) === 'white' /* force newline */
Expand All @@ -33,9 +25,7 @@ type SelectorProps = $ReadOnly<{|
|}>;

type Props = $ReadOnly<{
insets: EdgeInsets,

backgroundColor: string,
backgroundColor?: string | void,
hidden: boolean,

dispatch: Dispatch,
Expand All @@ -44,38 +34,34 @@ type Props = $ReadOnly<{

/**
* Applies `hidden` and `backgroundColor` in platform-specific ways.
*
* Like `StatusBar`, which is the only thing it ever renders, this
* doesn't have any effect on the spatial layout of the UI.
*/
class ZulipStatusBar extends PureComponent<Props> {
static defaultProps = {
hidden: false,
backgroundColor: DEFAULT_TITLE_BACKGROUND_COLOR,
};

render() {
const { theme, hidden, insets, orientation } = this.props;
const { theme, hidden, orientation } = this.props;
const backgroundColor = this.props.backgroundColor;
const style = { height: hidden ? 0 : insets.top, backgroundColor };
const statusBarColor = getStatusBarColor(backgroundColor, theme);
return (
orientation === 'PORTRAIT' && (
<View style={style}>
<StatusBar
animated
showHideTransition="slide"
hidden={hidden && Platform.OS !== 'android'}
backgroundColor={Color(statusBarColor).darken(0.1)}
barStyle={getStatusBarStyle(statusBarColor)}
/>
</View>
<StatusBar
animated
showHideTransition="slide"
hidden={hidden && Platform.OS !== 'android'}
backgroundColor={Color(statusBarColor).darken(0.1)}
barStyle={getStatusBarStyle(statusBarColor)}
/>
)
);
}
}

export default compose(
connect<SelectorProps, _, _>((state, props) => ({
theme: getSettings(state).theme,
orientation: getSession(state).orientation,
})),
withSafeAreaInsets,
)(ZulipStatusBar);
export default connect<SelectorProps, _, _>((state, props) => ({
theme: getSettings(state).theme,
orientation: getSession(state).orientation,
}))(ZulipStatusBar);
7 changes: 2 additions & 5 deletions src/common/__tests__/getStatusBarColor-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* @flow strict-local */
import { DEFAULT_TITLE_BACKGROUND_COLOR } from '../../title/titleSelectors';
import { getStatusBarColor } from '../ZulipStatusBar';

const themeNight = 'night';
Expand All @@ -12,9 +11,7 @@ describe('getStatusBarColor', () => {
});

test('returns color according to theme for default case', () => {
expect(getStatusBarColor(DEFAULT_TITLE_BACKGROUND_COLOR, themeDefault)).toEqual('white');
expect(getStatusBarColor(DEFAULT_TITLE_BACKGROUND_COLOR, themeNight)).toEqual(
'hsl(212, 28%, 18%)',
);
expect(getStatusBarColor(undefined, themeDefault)).toEqual('white');
expect(getStatusBarColor(undefined, themeNight)).toEqual('hsl(212, 28%, 18%)');
});
});
1 change: 1 addition & 0 deletions src/lightbox/LightboxScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type Props = $ReadOnly<{|

export default function LightboxScreen(props: Props) {
const { src, message } = props.route.params;

return (
<View style={styles.screen}>
<ZulipStatusBar hidden backgroundColor="black" />
Expand Down
5 changes: 4 additions & 1 deletion src/main/MainTabsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import {
createBottomTabNavigator,
type BottomTabNavigationProp,
} from '@react-navigation/bottom-tabs';
import { useSafeAreaInsets } from 'react-native-safe-area-context';

import type { RouteProp, RouteParamsOf } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import type { GlobalParamList } from '../nav/globalTypes';

import { bottomTabNavigatorConfig } from '../styles/tabs';
import HomeScreen from './HomeScreen';
import StreamTabsScreen from './StreamTabsScreen';
Expand Down Expand Up @@ -50,6 +50,8 @@ export default function MainTabsScreen(props: Props) {
const { backgroundColor } = useContext(ThemeContext);
const haveServerData = useSelector(getHaveServerData);

const insets = useSafeAreaInsets();

if (!haveServerData) {
// This can happen if the user has just logged out; this screen
// is still visible for the duration of the nav transition, and
Expand All @@ -63,6 +65,7 @@ export default function MainTabsScreen(props: Props) {

return (
<View style={[styles.flexed, { backgroundColor }]}>
<View style={{ height: insets.top }} />
<ZulipStatusBar />
<OfflineNotice />
<Tab.Navigator
Expand Down
43 changes: 17 additions & 26 deletions src/nav/ChatNavBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
import React from 'react';
import { View } from 'react-native';
import Color from 'color';
import { SafeAreaView } from 'react-native-safe-area-context';

import type { Narrow, EditMessage } from '../types';
import { LoadingBanner } from '../common';
import { useSelector } from '../react-redux';
import { BRAND_COLOR, NAVBAR_SIZE } from '../styles';
import Title from '../title/Title';
import NavBarBackButton from './NavBarBackButton';
import { DEFAULT_TITLE_BACKGROUND_COLOR, getTitleBackgroundColor } from '../title/titleSelectors';
import { getStreamColorForNarrow } from '../subscriptions/subscriptionSelectors';
import { foregroundColorFromBackground } from '../utils/color';
import { ExtraButton, InfoButton } from '../title-buttons/titleButtonFromNarrow';

Expand All @@ -21,46 +22,36 @@ type Props = $ReadOnly<{|

export default function ChatNavBar(props: Props) {
const { narrow, editMessage } = props;
const backgroundColor = useSelector(state => getTitleBackgroundColor(state, narrow));
const streamColor = useSelector(state => getStreamColorForNarrow(state, narrow));
const color =
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
? BRAND_COLOR
: foregroundColorFromBackground(backgroundColor);
streamColor === undefined ? BRAND_COLOR : foregroundColorFromBackground(streamColor);
const spinnerColor =
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
? 'default'
: foregroundColorFromBackground(backgroundColor);
streamColor === undefined ? 'default' : foregroundColorFromBackground(streamColor);

return (
<View
Comment on lines 31 to -35
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The default
algorithm used to display the diff in `ChatNavBar` gives a lot of
noise; `git diff -w` should reduce that quite a lot.)

It will, but perhaps this is a good time to plug a handy Git feature! This is one I relatively recently discovered and enabled in my gitconfig:

# Use color to identify code that was moved around verbatim.
[diff]
	# Enable the feature.
	colorMoved = zebra

	# Supercharge it to do the equivalent of `git diff -b`.
	# …
	colorMovedWS = ignore-space-change

(And git diff -b is a close relative of -w; they're pretty similar but I tend to find -b is closer to what I'm really looking for in principle.)

So here's how the main part of that diff comes out with my normal git usp, without going back and adding any other flags:

image

The "moved" colors let me see immediately that the code in each of those chunks didn't change beyond getting indented. Then I can match up the chunks by eye, and pretty quickly isolate what's changed. Not as simple as the git diff -b or -w view, but simple enough that I usually don't feel the need anymore to go reach for that one.

(The default colors are different from those and I think less helpful; those colors are also in my posted gitconfig.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, interesting!

Not as simple as the git diff -b or -w view, but simple enough that I usually don't feel the need anymore to go reach for that one.

It is quite a lot more lines than git diff -b. I'm not sure how long it'll take until I see this output as simpler, but I'll give it a go and find out! 🙂

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, the output of git diff -b is definitely simpler than this!

I just mean that this is simpler than reading the plain diff (and having to look closely to spot the few bits that changed); and it's enough simpler, giving me enough of what I'd get from git diff -b, that when I'm in the middle of a review I typically don't feel the need to go reach for git diff -b.

(And I don't just have -b or -w on all the time, because I do want to know about the changes those ignore; when they're relevant, I want to both see those changes, and see the changes with those simplified away, just separately.)

<SafeAreaView
mode="padding"
edges={['top', 'right', 'left']}
style={{
borderColor:
backgroundColor === 'transparent'
? 'hsla(0, 0%, 50%, 0.25)'
: Color(backgroundColor).darken(0.1),
streamColor === undefined ? 'hsla(0, 0%, 50%, 0.25)' : Color(streamColor).darken(0.1),
borderBottomWidth: 1,
backgroundColor: streamColor,
}}
>
<View
style={[
{
flexDirection: 'row',
height: NAVBAR_SIZE,
alignItems: 'center',
},
{ backgroundColor },
]}
style={{
flexDirection: 'row',
height: NAVBAR_SIZE,
alignItems: 'center',
}}
>
<NavBarBackButton color={color} />
<Title color={color} narrow={narrow} editMessage={editMessage} />
<ExtraButton color={color} narrow={narrow} />
<InfoButton color={color} narrow={narrow} />
</View>
<LoadingBanner
spinnerColor={spinnerColor}
backgroundColor={backgroundColor}
textColor={color}
/>
</View>
<LoadingBanner spinnerColor={spinnerColor} backgroundColor={streamColor} textColor={color} />
</SafeAreaView>
);
}
8 changes: 5 additions & 3 deletions src/nav/ModalNavBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import React, { useContext } from 'react';
import { View } from 'react-native';
import { SafeAreaView } from 'react-native-safe-area-context';

import type { LocalizableText } from '../types';
import styles, { ThemeContext, NAVBAR_SIZE } from '../styles';
Expand All @@ -22,12 +23,13 @@ export default function ModalNavBar(props: Props) {
];

return (
<View
<SafeAreaView
mode="padding"
edges={['top', 'right', 'left']}
style={[
{
borderColor: 'hsla(0, 0%, 50%, 0.25)',
flexDirection: 'row',
height: NAVBAR_SIZE,
alignItems: 'center',
borderBottomWidth: 1,
backgroundColor,
Expand All @@ -38,6 +40,6 @@ export default function ModalNavBar(props: Props) {
<View style={styles.flexedLeftAlign}>
<Label style={textStyle} text={title} numberOfLines={1} ellipsizeMode="tail" />
</View>
</View>
</SafeAreaView>
);
}
11 changes: 6 additions & 5 deletions src/nav/ModalSearchNavBar.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* @flow strict-local */
import React, { useContext } from 'react';
import { View } from 'react-native';
import { SafeAreaView } from 'react-native-safe-area-context';

import { ThemeContext, NAVBAR_SIZE } from '../styles';
import { ThemeContext } from '../styles';
import SearchInput from '../common/SearchInput';
import NavBarBackButton from './NavBarBackButton';

Expand All @@ -16,18 +16,19 @@ export default function ModalSearchNavBar(props: Props) {
const { autoFocus, searchBarOnChange, canGoBack = true } = props;
const { backgroundColor } = useContext(ThemeContext);
return (
<View
<SafeAreaView
mode="padding"
edges={['top', 'right', 'left']}
style={{
borderColor: 'hsla(0, 0%, 50%, 0.25)',
flexDirection: 'row',
height: NAVBAR_SIZE,
alignItems: 'center',
borderBottomWidth: 1,
backgroundColor,
}}
>
{canGoBack && <NavBarBackButton />}
<SearchInput autoFocus={autoFocus} onChangeText={searchBarOnChange} />
</View>
</SafeAreaView>
);
}
1 change: 0 additions & 1 deletion src/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export * from './emoji/emojiSelectors';
export * from './message/messageSelectors';
export * from './nav/navSelectors';
export * from './subscriptions/subscriptionSelectors';
export * from './title/titleSelectors';
export * from './topics/topicSelectors';
export * from './typing/typingSelectors';
export * from './unread/unreadSelectors';
Expand Down
2 changes: 0 additions & 2 deletions src/settings/SettingsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
IconLanguage,
IconMoreHorizontal,
} from '../common/Icons';
import ModalNavBar from '../nav/ModalNavBar';
import {
settingsChange,
navigateToNotifications,
Expand Down Expand Up @@ -52,7 +51,6 @@ class SettingsScreen extends PureComponent<Props> {

return (
<ScrollView style={styles.optionWrapper}>
<ModalNavBar canGoBack={false} title="Settings" />
Comment on lines 53 to -55
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh gosh this was inside the ScrollView! That never made any sense.

Which... oh, I see, this was quite noticeable on iOS.

On Android it could only come up if there was more there than fit vertically. I can make that happen by cranking up the system "Display size" setting plus going into landscape; but otherwise it's pretty unlikely, because we only have a few items on this settings screen.

But iOS has the animation where you can tug the scrollable content around even when there's no scrolling to be done, and the "Settings" header would move right along.

<OptionRow
Icon={IconNight}
label="Night mode"
Expand Down
9 changes: 9 additions & 0 deletions src/start/LoadingScreen.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* @flow strict-local */
import React from 'react';
import { View } from 'react-native';
import { useSafeAreaInsets } from 'react-native-safe-area-context';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
Expand All @@ -27,8 +28,16 @@ type Props = $ReadOnly<{|
|}>;

export default function LoadingScreen(props: Props) {
const insets = useSafeAreaInsets();

return (
<View style={componentStyles.center}>
<View
style={{
height: insets.top,
backgroundColor: BRAND_COLOR,
}}
/>
<ZulipStatusBar backgroundColor={BRAND_COLOR} />
<LoadingIndicator color="black" size={80} showLogo />
</View>
Expand Down
Loading