Page MenuHomePhabricator

[native] Fix CalendarInputBar
ClosedPublic

Authored by ashoat on May 31 2024, 7:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 6:10 AM
Unknown Object (File)
Sun, Nov 24, 9:46 PM
Unknown Object (File)
Sun, Nov 24, 1:41 PM
Unknown Object (File)
Sun, Nov 24, 6:42 AM
Unknown Object (File)
Thu, Nov 14, 3:53 AM
Unknown Object (File)
Wed, Nov 13, 3:41 PM
Unknown Object (File)
Tue, Nov 12, 2:29 PM
Unknown Object (File)
Tue, Nov 12, 12:27 PM
Subscribers

Details

Summary

CalendarInputBar was broken because the stack header was being rendered outside of KeyboardAvoidingView. This led to the wrong height being detected for the viewFrame inside of KeyboardAvoidingView, which resulted in CalendarInputBar not being moved high enough on the screen to be visible. (It was being rendered below the keyboard.)

This was first broken when we launched the navigation side drawer, since we needed to start rendering a header in Calendar so that there would be space for the "header left button" (the "hamburger" menu button for opening the navigation side drawer).

This diff solves the issue by wrapping the Calendar in a StackNavigator that contains only one route. This felt like overkill, but I wasn't able to get it to work by wrapping the Calendar screen directly (the header was still rendered outside) or by wrapping the component rendered by StackView (React Navigation requires that the children of Stack.Navigator are Stack.Screen).

Note that this diff transitions us from using KeyboardAvoidingView with behavior="position" to behavior="padding". I could deprecate the former, but have found that keeping our KeyboardAvoidingView aligned with upstream is a better strategy for being able to diff changes.

As a consequence of changing the behavior, I also needed to add height: 0 to CalendarInputButton when it's disabled. This makes the animation a little jarring, which I will address in the next diff.

Test Plan
  1. I added { borderColor: 'red', borderWidth: 2 } to KeyboardAvoidingView to see what it was wrapping and confirmed it now wraps the whole screen
  2. I logged the value of this.viewFrame in KeyboardAvoidingView to make sure it matched what we had in other places now (Chat and Profile)
  3. I played around on my local iOS simulator to confirm that the CalendarInputBar now appears above the keyboard

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/calendar/calendar-input-bar.react.js
35 ↗(On Diff #40806)

From the diff description:

As a consequence of changing the behavior, I also needed to add height: 0 to CalendarInputButton when it's disabled. This makes the animation a little jarring, which I will address in the next diff.

native/calendar/calendar.react.js
63 ↗(On Diff #40806)

Why is this needed?

native/calendar/calendar.react.js
63 ↗(On Diff #40806)

There's only one route right now, so it shouldn't matter. I can remove if if you'd like. It looks like this is included in the Profile navigator, which I initially modelled this navigator after. For Chat and RootNavigator, we set it for Android but not for iOS. Not sure why our approach is inconsistent.

More details about the param here.

Remove detachInactiveScreens={false}

This revision is now accepted and ready to land.Jun 4 2024, 12:59 AM
This revision was automatically updated to reflect the committed changes.