Skip to content

Commit

Permalink
nav [nfc]: Remove DEFAULT_TITLE_BACKGROUND_COLOR.
Browse files Browse the repository at this point in the history
We're only using this as a sentinel value [1], so `undefined` works
just as well and is less to think about.

[1] zulip#4443 (comment)
  • Loading branch information
chrisbobbe committed Jan 30, 2021
1 parent 3ac4225 commit f3eaab0
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 28 deletions.
9 changes: 4 additions & 5 deletions src/common/ZulipStatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import Color from 'color';

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
export const getStatusBarColor = (backgroundColor: string | void, theme: ThemeName): string =>
backgroundColor === undefined
? theme === 'night'
? 'hsl(212, 28%, 18%)'
: 'white'
Expand All @@ -30,7 +29,7 @@ type SelectorProps = $ReadOnly<{|
|}>;

type Props = $ReadOnly<{
backgroundColor: string,
backgroundColor: string | void,
hidden: boolean,

dispatch: Dispatch,
Expand All @@ -46,7 +45,7 @@ type Props = $ReadOnly<{
class ZulipStatusBar extends PureComponent<Props> {
static defaultProps = {
hidden: false,
backgroundColor: DEFAULT_TITLE_BACKGROUND_COLOR,
backgroundColor: undefined,
};

render() {
Expand Down
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%)');
});
});
15 changes: 5 additions & 10 deletions src/nav/ChatNavBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ 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 { getTitleBackgroundColor } from '../title/titleSelectors';
import { foregroundColorFromBackground } from '../utils/color';
import { ExtraButton, InfoButton } from '../title-buttons/titleButtonFromNarrow';

Expand All @@ -24,26 +24,21 @@ export default function ChatNavBar(props: Props) {
const { narrow, editMessage } = props;
const backgroundColor = useSelector(state => getTitleBackgroundColor(state, narrow));
const color =
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
? BRAND_COLOR
: foregroundColorFromBackground(backgroundColor);
backgroundColor === undefined ? BRAND_COLOR : foregroundColorFromBackground(backgroundColor);
const spinnerColor =
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
? 'default'
: foregroundColorFromBackground(backgroundColor);
backgroundColor === undefined ? 'default' : foregroundColorFromBackground(backgroundColor);

return (
<SafeAreaView
mode="padding"
edges={['top', 'right', 'left']}
style={{
borderColor:
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR
backgroundColor === undefined
? 'hsla(0, 0%, 50%, 0.25)'
: Color(backgroundColor).darken(0.1),
borderBottomWidth: 1,
backgroundColor:
backgroundColor === DEFAULT_TITLE_BACKGROUND_COLOR ? undefined : backgroundColor,
backgroundColor: backgroundColor === undefined ? undefined : backgroundColor,
}}
>
<View
Expand Down
8 changes: 3 additions & 5 deletions src/title/__tests__/titleSelectors-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @flow strict-local */

import { DEFAULT_TITLE_BACKGROUND_COLOR, getTitleBackgroundColor } from '../titleSelectors';
import { getTitleBackgroundColor } from '../titleSelectors';
import { pmNarrowFromUsersUnsafe, streamNarrow, pm1to1NarrowFromUser } from '../../utils/narrow';
import * as eg from '../../__tests__/lib/exampleData';

Expand All @@ -20,11 +20,9 @@ describe('getTitleBackgroundColor', () => {
});

test('return default for non topic/stream narrow', () => {
expect(getTitleBackgroundColor(state, pm1to1NarrowFromUser(eg.otherUser))).toEqual(
DEFAULT_TITLE_BACKGROUND_COLOR,
);
expect(getTitleBackgroundColor(state, pm1to1NarrowFromUser(eg.otherUser))).toEqual(undefined);
expect(
getTitleBackgroundColor(state, pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser])),
).toEqual(DEFAULT_TITLE_BACKGROUND_COLOR);
).toEqual(undefined);
});
});
4 changes: 1 addition & 3 deletions src/title/titleSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import type { Narrow, GlobalState } from '../types';
import { isStreamOrTopicNarrow, streamNameOfNarrow } from '../utils/narrow';
import { getSubscriptionsByName } from '../subscriptions/subscriptionSelectors';

export const DEFAULT_TITLE_BACKGROUND_COLOR = 'transparent';

/**
* Background color to use for the app bar in narrow `narrow`.
*
Expand All @@ -14,7 +12,7 @@ export const DEFAULT_TITLE_BACKGROUND_COLOR = 'transparent';
export const getTitleBackgroundColor = (state: GlobalState, narrow: Narrow) => {
const subscriptionsByName = getSubscriptionsByName(state);
if (!isStreamOrTopicNarrow(narrow)) {
return DEFAULT_TITLE_BACKGROUND_COLOR;
return undefined;
}
const streamName = streamNameOfNarrow(narrow);
return subscriptionsByName.get(streamName)?.color ?? 'gray';
Expand Down

0 comments on commit f3eaab0

Please sign in to comment.