Clean up navigation logic (3/x). #4443

chrisbobbe · 2021-01-29T00:39:28Z

Following #4428 and #4440, this PR takes the job of handling the top inset away from ZulipStatusBar.

It doesn't yet establish a consistent scheme for how we should handle safe areas (for #3066 and put a ratchet on a fix for it), but it brings us closer to being able to do that, and I aim to do that in a future PR. 🙂

My goal for that scheme is this:

Each screen on a stack navigator should use its header prop to either
- Show a custom, non-trivial header, e.g., with nav buttons, etc., that avoids the top inset. Its content should also be padded on the left and right to avoid the horizontal insets when in landscape mode. (ChatNavBar, ModalNavBar, etc., can pretty much be dropped in here after this PR.) Or,
- Fill the top inset, and that's it.
- (React Navigation is happy to provide a default, batteries-included header that handles safe areas and shows the current route name and a back button, etc.—but we've so far avoided that, and the default styling doesn't really work with our app. Still, even with custom headers, I'm expecting the use of the header prop to help make consistency across our headers more natural than it is now.)
- Special notes about MainTabsScreen:
  - MainTabsScreen itself is a screen on a stack navigator, AppNavigator. It'll get the latter treatment; just a simple View that covers the top inset.
  - The tab screens directly on MainTabsScreen aren't configurable with a header prop. They fill the entire area between the bottom tab bar and MainTabsScreen's trivial space-covering header (just mentioned). So they just need to worry about their horizontal padding.
  - The bottom tab bar already avoids the bottom inset because of some logic under the hood, I think.
  - If we were planning to put a stack navigator on each tab (or some tabs), we'd have to think about double headers stacking on each other: (1) MainTabsScreen's header, and (2) the header for each screen on such a stack navigator. (Likely meaning handling the top inset twice, which is one too many times.) But we're not currently planning to use this pattern.
Each screen component on a stack navigator is responsible for keeping important content out of the right, bottom, and left insets. It doesn't have to worry about the top inset because its header does that, as mentioned above.

Linking React Navigation's doc on handling safe areas.

gnprice · 2021-01-29T01:53:09Z

(I'll look at the actual content here in a bit, but:

It doesn't yet […] fix #3066

this is a classic example of the kind of phrasing that trips up GitHub's "this is supposed to fix that" detector 😉. Should reword to either not say "fix", or squeeze a word in between "fix" and the issue number.)

chrisbobbe · 2021-01-29T20:49:33Z

Ah, right; thanks for catching that.

gnprice

Thanks @chrisbobbe! Glad to see all these things straightened out. Some comments below.

My goal for that scheme is this:

This plan sounds good to me.

gnprice · 2021-01-29T21:56:16Z

src/common/ZulipStatusBar.js

+        <>
+          <View style={style} />
          <StatusBar


React Native's `StatusBar` component is unusual in that its `render` method always returns null [1]. The component isn't meant to participate in the spatial layout of the UI elements. Instead, the API works more like this: Mount a `StatusBar` component in order to tell something new to the native status bar API; e.g., what colors and animation you want to use.

Interesting, thanks for sorting this out!

src/start/LoadingScreen.js

gnprice · 2021-01-29T22:11:21Z

src/main/MainTabsScreen.js

-        <View
-          style={{
-            height: insets.top,
-            backgroundColor: DEFAULT_TITLE_BACKGROUND_COLOR,


The value of this constant is `'transparent'`, which is the default background color of `View`s anyway.

It looks like after this change a further simplification is possible: I believe it's the case that DEFAULT_TITLE_BACKGROUND_COLOR is only ever used as a sentinel value! That is, it signifies to our own code "there's nothing here, just use your default behavior", and we have conditionals that test for equality to it... but I believe we never pass it as an actual style prop, or otherwise to any code outside our own.

If so, that means that we could replace the constant's initializer with undefined, or for that matter {} or anything that makes a fresh unique object. And probably better -- we could just replace every use of it with undefined, and augment the relative types with | void to make that possibility explicit.

Hmm, interesting! I think I see just one place where we pass it to code that's not ours, at this commit:

src/chat/ChatScreen

const titleBackgroundColor = useSelector(state => getTitleBackgroundColor(state, narrow)); // … return ( <ActionSheetProvider> <View style={[componentStyles.screen, { backgroundColor }]}> <KeyboardAvoider style={styles.flexed} behavior="padding"> {orientation === 'PORTRAIT' && ( <View style={{ height: insets.top, backgroundColor: titleBackgroundColor, }} /> )}

getTitleBackgroundColor will return DEFAULT_TITLE_BACKGROUND_COLOR if the narrow is not a stream or topic narrow.

Ah, right—but passing undefined would be just the same as passing 'transparent' there.

My new revision makes DEFAULT_TITLE_BACKGROUND_COLOR go away. I've also renamed getTitleBackgroundColor to getStreamColorForNarrow, but I've kept the part of its interface that says it accepts narrows other than stream or topic narrows (for which it returns undefined). The react-hooks/rules-of-hooks rule doesn't like it if I conditionally use a hook, as in

const backgroundColor = isStreamNarrow(narrow) ? useSelector(state => getStreamColorForNarrow(state, narrow)) : '…';

React Hook "useSelector" is called conditionally. React Hooks must be called in the exact same order in every component render. (eslintreact-hooks/rules-of-hooks)

gnprice · 2021-01-29T22:13:12Z

src/chat/ChatScreen.js

  const insets = useSafeAreaInsets();

  return (
    <ActionSheetProvider>
      <View style={[componentStyles.screen, { backgroundColor }]}>
        <KeyboardAvoider style={styles.flexed} behavior="padding">
-          {orientation === 'PORTRAIT' && (


I can't think of a device that has a nonzero top inset in landscape mode, so it's possible that this commit is NFC. Still, we might as well (1) handle that case, so we don't have to think about it, and (2) remove something that can cause confusion.

Hmm, maybe a foldable? Or some odd-shaped tablet?

In any case I definitely agree with this reasoning.

gnprice · 2021-01-29T22:26:33Z

src/nav/ChatNavBar.js

  return (
-    <View


(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:

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.)

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! 🙂

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.)

gnprice · 2021-01-29T22:42:00Z

src/settings/SettingsScreen.js

      <ScrollView style={styles.optionWrapper}>
-        <ModalNavBar canGoBack={false} title="Settings" />


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.

gnprice · 2021-01-29T22:45:25Z

src/nav/ModalNavBar.js

-    <>
-      <View style={{ height: insets.top }} />
+    <View style={{ borderBottomWidth: 1, backgroundColor, paddingTop: insets.top }}>
      <View
        style={[
          {
            borderColor: 'hsla(0, 0%, 50%, 0.25)',
            flexDirection: 'row',
            height: NAVBAR_SIZE,
            alignItems: 'center',
-            borderBottomWidth: 1,
-            backgroundColor,


Hmm, is this quite right? The borderColor is getting separated from the borderBottomWidth.

For that matter: is there a reason not to collapse these into a single wrapper View? I don't think I'm spotting a way that we're using the distinction.

I guess the next commit turns the outer one into a SafeAreaView. But that's supposed to be just like a View with one extra feature, so I think the same question applies.

is there a reason not to collapse these into a single wrapper View? I don't think I'm spotting a way that we're using the distinction.

The existing View sets an explicit height of NAVBAR_SIZE (58), so the part that holds the content gets shrunk. Though I guess we could get rid of that explicit height; we recently removed one in #4442.

I think the situation is similar with ModalSearchNavBar.

Ah I see, because the padding comes out of the height.

We could also handle that directly, I think, by adding the desired padding to the height when combining these two Views:

paddingTop: insets.top, height: insets.top + NAVBAR_SIZE,

But removing NAVBAR_SIZE sounds good in any case.

We could also handle that directly, I think, by adding the desired padding to the height when combining these two Views:

paddingTop: insets.top, height: insets.top + NAVBAR_SIZE,

Oh, but this gets stickier when converting to SafeAreaView, doesn't it.

Anyway, happy to remove NAVBAR_SIZE 🙂

This is pretty clearly what was intended. Soon, though, we'll simplify things by removing the variable -- at this point, it's just being used as a sentinel value [1]. [1] zulip#4443 (comment)

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)

chrisbobbe · 2021-01-30T22:57:20Z

Thanks for the review! Revision pushed.

src/nav/ModalNavBar.js

gnprice · 2021-02-01T22:31:09Z

src/nav/ModalNavBar.js

-    <>
-      <View style={{ height: insets.top }} />
+    <View style={{ borderBottomWidth: 1, backgroundColor, paddingTop: insets.top }}>
      <View
        style={[
          {
            borderColor: 'hsla(0, 0%, 50%, 0.25)',
            flexDirection: 'row',
            height: NAVBAR_SIZE,
            alignItems: 'center',
-            borderBottomWidth: 1,
-            backgroundColor,


We could also handle that directly, I think, by adding the desired padding to the height when combining these two Views:

paddingTop: insets.top, height: insets.top + NAVBAR_SIZE,

Oh, but this gets stickier when converting to SafeAreaView, doesn't it.

Anyway, happy to remove NAVBAR_SIZE 🙂

gnprice

Thanks for the revision! I like the new further cleanup. Small comments above and below.

gnprice · 2021-02-01T22:40:02Z

src/title/titleSelectors.js

 /**
- * Background color to use for the app bar in narrow `narrow`.
+ * The stream's color for the given stream or topic narrow.
 *
- * If `narrow` is a stream or topic narrow, this is based on the stream color.
- * Otherwise, it takes a default value.
+ * Gives undefined for narrows that are not stream or topic narrows.
 */
-export const getTitleBackgroundColor = (state: GlobalState, narrow: Narrow) => {
+export const getStreamColorForNarrow = (state: GlobalState, narrow: Narrow) => {


With its new clearer name, I think this also is best moved to a different file: subscriptionSelectors.js, where it will fit right in.

That also lets us remove titleSelectors.js, which never made as much sense as the selectors files like subscriptionSelectors.js that are clearly about a particular part of the app's data model.

gnprice · 2021-02-01T22:41:05Z

src/common/ZulipStatusBar.js

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


This can also be dropped (probably in a small followup commit, like you've done already for similar cleanups.)

React Native's `StatusBar` component is unusual in that its `render` method always returns null [1]. The component isn't meant to participate in the spatial layout of the UI elements. Instead, the API works more like this: Mount a `StatusBar` component in order to tell something new to the native status bar API; e.g., what colors and animation you want to use. This commit is therefore NFC: the `StatusBar` still gets mounted in all cases where it was getting mounted before, and nothing changes in the spatial layout of the UI elements. The `<View style={style} />` is doing an important job: it covers the "safe area" at the top of the screen (which includes the status bar), so that our content doesn't accidentally overlap with it. But `ZulipStatusBar` already has a very different job, which is to configure the status bar in kind of subtle ways, and depending on the platform. I'd like to pull the safe-area handling out of `ZulipStatusBar` to give it one less thing to worry about; that will happen in the rest of this series of commits. [1] https://github.com/facebook/react-native/blob/v0.63.4/Libraries/Components/StatusBar/StatusBar.js#L497

We'll use this soon.

Now, `ZulipStatusBar` doesn't have to worry about handling safe area views, and we can tailor that logic more specifically to each of the callers.

Simplify some trivial expressions.

The value of this constant is `'transparent'`, which is the default background color of `View`s anyway.

In each of these instances, the `View` will be rendered at the top of the screen -- regardless of the device's orientation. (I.e., if it's in a portrait orientation, it'll be on a short side at the top; if it's in a landscape orientation, it'll be on a long side at the top.) The `View` is meant to guard against a potential unsafe area at the top. In principle, that isn't less necessary when we're in a landscape orientation. I can't think of a device that has a nonzero top inset in landscape mode, so it's possible that this commit is NFC. Still, we might as well (1) handle that case, so we don't have to think about it, and (2) remove something that can cause confusion.

As noted in a recent commit, the `ZulipStatusBar` component no longer has any effect on the spatial layout of the UI, so we're free to do this.

The nav bar is always present, for `ChatScreen`, and always at the top. So it's harmless to have the nav bar component also take control of handling the upper safe-area inset. (The default algorithm used to display the diff in `ChatNavBar` gives a lot of noise; `git diff -w` should reduce that quite a lot.) Soon, I'd like to start reversing our avoidance of React Navigation's screen-header APIs; i.e., it'll make sense for the `<Stack.Screen` with `component={ChatScreen}` to use `ChatNavBar` (or something much like it) in its `header` prop [1], instead of sticking it at the top of the screen component, where the screen's content belongs. When we do that, `ChatScreen` will properly describe only the content of the screen, below the header, and it'll be too far down the screen for top-safe-area handling to be done there. [1] https://reactnavigation.org/docs/stack-navigator/#header

react-native-safe-area-context recommends [1] using its `SafeAreaView` component, which applies the insets natively, to avoid flickering when you rotate the screen. [1] https://github.com/th3rdwave/react-native-safe-area-context

The contents of this header extend all the way to the left and the right of the screen. When in landscape mode, the left and right insets can be nonzero. We shouldn't add padding for the bottom inset because the header isn't at the bottom of the screen.

- None of the other tabs have a header like this, so it looks a bit odd and inconsistent. - It duplicates what's being said by the selected gear icon in the bottom tab bar. - It takes up space. See also discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Stack.20nav.20for.20settings.20tab/near/1107707.

As noted in a recent commit, the `ZulipStatusBar` component no longer has any effect on the spatial layout of the UI, so we're free to do this.

Like we did in a recent commit, moving a similar `View` from `ChatScreen` into `ChatNavBar`.

This limits our flexibility for gracefully handling the user's system font-size preference.

Now, `backgroundColor` extends up through the top inset.

react-native-safe-area-context recommends [1] using its `SafeAreaView` component, which applies the insets natively, to avoid flickering when you rotate the screen. [1] https://github.com/th3rdwave/react-native-safe-area-context

For landscape mode.

This limits our flexibility for gracefully handling the user's system font-size preference.

Now, `backgroundColor` extends up through the top inset.

For landscape mode.

This should be equivalent, and it'll help with a simplification we're about to do.

This is pretty clearly what was intended. Soon, though, we'll simplify things by removing the variable -- at this point, it's just being used as a sentinel value [1]. [1] zulip#4443 (comment)

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)

Some callers pass a `string` type; some don't pass anything, and one passes a `string | void` type. So `backgroundColor?: string | void` seems the most correct, even though we could probably get away with removing the `| void`.

Based on the rename of `getStreamColorForNarrow` (previously `getTitleBackgroundColor`) in a recent commit.

…tors. As Greg points out [1], this "never made as much sense as the selectors files like `subscriptionSelectors.js` that are clearly about a particular part of the app's data model." [1] zulip#4443 (comment)

chrisbobbe · 2021-02-01T23:21:02Z

Thanks for the review! Revision pushed.

gnprice · 2021-02-02T00:12:42Z

All looks good, thanks! Merged.

This is pretty clearly what was intended. Soon, though, we'll simplify things by removing the variable -- at this point, it's just being used as a sentinel value [1]. [1] zulip#4443 (comment)

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)

…tors. As Greg points out [1], this "never made as much sense as the selectors files like `subscriptionSelectors.js` that are clearly about a particular part of the app's data model." [1] zulip#4443 (comment)

These app bars -- all our app bars outside the message list, which uses ChatNavBar -- didn't have anything to actually specify how tall they should be. When the back button is present, it serves as a strut to keep the app bar at least as tall as the button is, which gets us pretty close: just a fraction of a pixel off in my testing on an Android 11 emulator. (In fact I don't understand why it isn't exactly right: `NavButtonGeneral` sets the height to NAVBAR_SIZE.) But when the back button is not present, the app bar would collapse to the height of the text, which is typically much shorter. Fix both cases by simply specifying the intended height, as a minimum. We actually used to have explicit heights like these, as simply `height: NAVBAR_SIZE`, and removed them a little while ago in zulip#4443 in commits 635b13a and a9ab3fc. The stated reason was to better handle large font sizes; it also simplified rearranging things for getting these `SafeAreaView`s in place. But those have settled down, and by using `minHeight` there's no trouble if the user has a very large font-size setting. (Also I don't think we noticed at the time that this had a large visible effect in the no-back-button case; if we had, we'd have done something about it then.)

These app bars -- all our app bars outside the message list, which uses ChatNavBar -- didn't have anything to actually specify how tall they should be. When the back button is present, it serves as a strut to keep the app bar at least as tall as the button is, which gets us pretty close: just a fraction of a pixel off in my testing on an Android 11 emulator. (In fact I don't understand why it isn't exactly right: `NavButtonGeneral` sets the height to NAVBAR_SIZE.) But when the back button is not present, the app bar would collapse to the height of the text, which is typically much shorter. Fix both cases by simply specifying the intended height, as a minimum. We actually used to have explicit heights like these, as simply `height: NAVBAR_SIZE`, and removed them a little while ago in zulip#4443 in commits 635b13a and a9ab3fc. The stated reason was to better handle large font sizes; it also simplified rearranging things for getting these `SafeAreaView`s in place. But those have settled down, and by using `minHeight` there's no trouble if the user has a very large font-size setting. (Also I don't think we noticed at the time that this had a large visible effect in the no-back-button case; if we had, we'd have done something about it then.) In the case of `ChatNavBar`, we do specify the height, and ideally we'd say `minHeight` instead there too. But that causes hilariously wrong results, presumably due to a bug somewhere else in our layout. For the present, just leave a TODO comment there.

chrisbobbe requested a review from gnprice January 29, 2021 00:39

karanb1 mentioned this pull request Jan 29, 2021

UserStatusScreen: Convert to use hooks & react navigation v5 #4447

Open

gnprice reviewed Jan 29, 2021

View reviewed changes

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jan 30, 2021

nav [nfc]: Remove DEFAULT_TITLE_BACKGROUND_COLOR.

f3eaab0

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)

chrisbobbe force-pushed the pr-nav-cleanup-3 branch from f02fe66 to f2d629d Compare January 30, 2021 22:57

gnprice reviewed Feb 1, 2021

View reviewed changes

chrisbobbe added 19 commits February 1, 2021 17:54

Screen [nfc]: Grab orientation from Redux.

fc08d86

We'll use this soon.

ZulipStatusBar [nfc]: Move sized View out to callers.

6956fc0

Now, `ZulipStatusBar` doesn't have to worry about handling safe area views, and we can tailor that logic more specifically to each of the callers.

ZulipStatusBar [nfc]: Settle into a recent change (1/x).

bac28e4

Simplify some trivial expressions.

ZulipStatusBar [nfc]: Settle into a recent change (2/x).

59a2d4c

ZulipStatusBar [nfc]: Settle into a recent change (3/x).

c1ab47e

The value of this constant is `'transparent'`, which is the default background color of `View`s anyway.

LightboxScreen [nfc]: Remove an invisible, zero-height View.

919c71d

ChatScreen [nfc]: Reorder View with ZulipStatusBar.

e7520ac

As noted in a recent commit, the `ZulipStatusBar` component no longer has any effect on the spatial layout of the UI, so we're free to do this.

ChatNavBar [nfc]: Better integrate safe-area handling (1/x).

84c7088

ChatNavBar [nfc]: Better integrate safe-area handling (2/x).

2f02651

Screen [nfc]: Reorder View with ZulipStatusBar.

08c1904

As noted in a recent commit, the `ZulipStatusBar` component no longer has any effect on the spatial layout of the UI, so we're free to do this.

Screen [nfc]: Move safe-area-handling View into Modal(Search)NavBar.

746a9f6

Like we did in a recent commit, moving a similar `View` from `ChatScreen` into `ChatNavBar`.

ModalNavBar: Remove explicit height.

635b13a

This limits our flexibility for gracefully handling the user's system font-size preference.

ModalNavBar: Better integrate safe-area handling (1/x).

3f5bcf3

Now, `backgroundColor` extends up through the top inset.

chrisbobbe added 15 commits February 1, 2021 18:00

ModalNavBar: Handle left and right insets too.

fd81cc7

For landscape mode.

ModalSearchNavBar: Remove explicit height.

a9ab3fc

This limits our flexibility for gracefully handling the user's system font-size preference.

ModalSearchNavBar: Better integrate safe-area handling (1/x).

93d9efa

Now, `backgroundColor` extends up through the top inset.

ModalSearchNavBar: Better integrate safe-area handling (2/x).

d1fd30d

ModalSearchNavBar: Handle left and right insets too.

deb3015

For landscape mode.

ChatNavBar [nfc]: Pass undefined instead of 'transparent'.

26f6f23

This should be equivalent, and it'll help with a simplification we're about to do.

ChatNavBar [nfc]: Use a variable instead of 'transparent'.

5abde56

This is pretty clearly what was intended. Soon, though, we'll simplify things by removing the variable -- at this point, it's just being used as a sentinel value [1]. [1] zulip#4443 (comment)

nav [nfc]: Remove DEFAULT_TITLE_BACKGROUND_COLOR.

a9c1b44

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)

nav [nfc]: Simplify a few conditionals.

42396b6

titleSelectors [nfc]: Give getTitleBackgroundColor a better name.

dada934

titleSelectors [nfc]: Make an early return earlier.

6782cfd

nav [nfc]: Improve a few variable names.

2288807

Based on the rename of `getStreamColorForNarrow` (previously `getTitleBackgroundColor`) in a recent commit.

titleSelectors [nfc]: Delete; move only selector to subscriptionSelec…

0a218e0

…tors. As Greg points out [1], this "never made as much sense as the selectors files like `subscriptionSelectors.js` that are clearly about a particular part of the app's data model." [1] zulip#4443 (comment)

chrisbobbe force-pushed the pr-nav-cleanup-3 branch from f2d629d to 0a218e0 Compare February 1, 2021 23:21

gnprice merged commit 0a218e0 into zulip:master Feb 2, 2021

chrisbobbe mentioned this pull request Feb 4, 2021

Clean up navigation logic (4/x): Tidy up our use of ZulipStatusBar a bit more. #4468

Merged

chrisbobbe deleted the pr-nav-cleanup-3 branch November 4, 2021 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up navigation logic (3/x). #4443

Clean up navigation logic (3/x). #4443

chrisbobbe commented Jan 29, 2021 •

edited

Loading

gnprice commented Jan 29, 2021

chrisbobbe commented Jan 29, 2021

gnprice left a comment

gnprice Jan 29, 2021

gnprice Jan 29, 2021

chrisbobbe Jan 30, 2021

chrisbobbe Jan 30, 2021

chrisbobbe Jan 30, 2021 •

edited

Loading

gnprice Jan 29, 2021

gnprice Jan 29, 2021

chrisbobbe Jan 30, 2021

gnprice Feb 1, 2021

gnprice Jan 29, 2021

gnprice Jan 29, 2021

gnprice Jan 29, 2021

chrisbobbe Jan 30, 2021

chrisbobbe Jan 30, 2021

gnprice Feb 1, 2021

gnprice Feb 1, 2021

chrisbobbe commented Jan 30, 2021

gnprice Feb 1, 2021

gnprice left a comment

gnprice Feb 1, 2021

gnprice Feb 1, 2021

chrisbobbe commented Feb 1, 2021

gnprice commented Feb 2, 2021

		<ScrollView style={styles.optionWrapper}>
		<ModalNavBar canGoBack={false} title="Settings" />

Clean up navigation logic (3/x). #4443

Clean up navigation logic (3/x). #4443

Conversation

chrisbobbe commented Jan 29, 2021 • edited Loading

gnprice commented Jan 29, 2021

chrisbobbe commented Jan 29, 2021

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe Jan 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Jan 30, 2021

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Feb 1, 2021

gnprice commented Feb 2, 2021

chrisbobbe commented Jan 29, 2021 •

edited

Loading

chrisbobbe Jan 30, 2021 •

edited

Loading