No margin for main bottom nav on iPhone 12 #4369
Comments
|
@gnprice Can I work on this issue? |
Gautam-Arora24
added a commit
to Gautam-Arora24/zulip-mobile
that referenced
this issue
Jan 12, 2021
Fixed the margin problem on iPhone 12 models by updating the react-native-safe-area-context and react-navigation-tabs dependency Fixes:zulip#4369
Gautam-Arora24
added a commit
to Gautam-Arora24/zulip-mobile
that referenced
this issue
Jan 12, 2021
Fixed the margin issue by upgrading the react-navigation-tabs dependency Fixes: zulip#4369
chrisbobbe
referenced
this issue
Jan 14, 2021
Done mostly with the upgrade guide at https://reactnavigation.org/docs/upgrading-from-4.x, but also by consulting the v4 and v5 documentation of specific areas. In this commit: - Take the packages' name changes (e.g., react-navigation -> @react-navigation/native) - Add react-native-tab-view to handle a peer-dep warning: warning " > @react-navigation/[email protected]" has unmet peer dependency "react-native-tab-view@>= 2.0.0". - Run the usual `yarn yarn-deduplicate && yarn`. - Use the newly available `NavigationContainer` instead of the removed `createAppContainer`. We leave it in its own file (src/nav/Zulip { App -> Navigation } Container.js), since it still has to handle some special logic: - setting up the NavigationService with a ref (the initialization gets slightly more complicated, with the `onReady` prop) - deciding what to pass for the `theme` prop, based on the current theme (for more discussion of our use of this prop, see the recent commit where we started passing it) - Update `NavigationService` to handle the new shape of the thing it tracks with a ref (`AppContainer` -> `NavigationContainer`), and handle some new initialization logic for `NavigationContainer` (its `onReady` prop). - Change `MessageReactionList` over to the new component-based API [1], without going through @react-navigation/compat first. (It's small and self-contained.) - Undo f4b8253, i.e., stop threading through `navigation` prop and assigning to `static router` in MainScreenWithTabs. The "common mistakes" doc that prescribed those things has been removed in v5, and, empirically, everything works fine without it. - Use @react-navigation/compat to reduce the size of this commit (we'll incrementally move away from it in the upcoming commits). (Note that that package is completely untyped [2] -- this causes Flow to misleadingly mark some `$FlowFixMe`s unnecessary; still, we go along with Flow and remove them -- but once we've completely adopted the v5 APIs, our types should be stronger than before.) There are a few things we use the compat package for [3]: - Postpone changes to the definitions of our four navigators: `AppNavigator`, `MainTabs`, `StreamTabs`, and `SharingScreen`, with `createCompatNavigatorFactory`. - In `navActions`, postpone taking the new interfaces of various action creators (`StackActions.push`, etc.). This isn't possible with `.reset` actions, but we only use three of those, so we change them in this commit. - Keep using the `withNavigationFocus` HOC, so we can switch to the new custom Hook later. And a few minor naming / simple API tweaks: - In `SmartUrlInput`, use the 'focus' event type instead of the now-removed one, 'didFocus', and adapt to the new API for removing a listener. - What used to be called `routeName` on a route in the navigation state is now called `name`. - Even though we're using `createCompatNavigatorFactory`, we have to access `tintColor` instead of `color` on the object passed to the function we pass to `tabBarLabel` and `tabBarIcon` in a route config on a tab navigator. - Various types are renamed, like `NavigationParams` -> `ScreenParams`. - Apparently `upperCaseLabel` in a tab navigator's config...vanished in this new version. Add a TODO with findings from a GitHub discussion about this [4]. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/React.20Navigation.20v5.3A.20MessageReactionList/near/1061924 [2] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/React.20Navigation.20v5/near/1046571 [3] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/React.20Navigation.20v5.3A.20compat/near/1058109 [4] #4393 (comment)
|
This was fixed in 1228450; see my comment at 1228450#commitcomment-45962457. Thanks for the PR, @Gautam-Arora24, and again, I'm sorry for this mistake. |
|
No worries @chrisbobbe Will start working on another issue 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@titanous reports on #4365:
and adds at #4365 (comment) :
The symptom is reminiscent of #3066. It seems like there's a new behavior with the layout of the iPhone 12 screen (but not the 12 mini), which we're not handling properly.
As for why local builds differ from the version in the App Store -- those local builds were with Xcode 12.3, whereas the latest App Store version was built with Xcode 11.7 (or possibly earlier). For the main issue of #4365, @titanous suggests
and that seems quite plausible here too -- i.e. perhaps as long as the app was built with Xcode 11, the iPhone 12 doesn't expect it to know about the iPhone 12's new behavior, and treats it with a compatibility mode.
The text was updated successfully, but these errors were encountered: