forked from zulip/zulip-mobile
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
KeyboardAvoider: Reimplement iOS logic to abandon RN's KeyboardAvoidi…
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
- Loading branch information
1 parent
8c91495
commit 546b8a4
Showing
4 changed files
with
181 additions
and
42 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,58 +1,27 @@ | ||
/* @flow strict-local */ | ||
import React, { PureComponent } from 'react'; | ||
import type { Node } from 'react'; | ||
import { KeyboardAvoidingView, Platform, View } from 'react-native'; | ||
import { Platform, View } from 'react-native'; | ||
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; | ||
|
||
import KeyboardAvoiderIos from './KeyboardAvoiderIos'; | ||
|
||
type Props = $ReadOnly<{| | ||
behavior?: ?('height' | 'position' | 'padding'), | ||
children: Node, | ||
style?: ViewStyleProp, | ||
|
||
/** How much the top of `KeyboardAvoider`'s layout *parent* is | ||
* displaced downward from the top of the screen. | ||
* | ||
* If this isn't set correctly, the keyboard will hide some of | ||
* the bottom of the screen (an area whose height is what this | ||
* value should have been set to). | ||
* | ||
* I think `KeyboardAvoidingView`'s implementation mistakes `x` | ||
* and `y` from `View#onLayout` to be a `View`'s position | ||
* relative to the top left of the screen. In reality, I'm | ||
* pretty sure they represent a `View`'s position relative to | ||
* its parent: | ||
* https://github.com/facebook/react-native-website/issues/2056#issuecomment-773618381 | ||
* | ||
* But at least `KeyboardAvoidingView` exposes this prop, which | ||
* we can use to balance the equation if we need to. | ||
*/ | ||
keyboardVerticalOffset?: number, | ||
|}>; | ||
|
||
/** | ||
* Renders RN's `KeyboardAvoidingView` on iOS, `View` on Android. | ||
* | ||
* This component's props that are named after | ||
* `KeyboardAvoidingView`'s special props get passed straight through | ||
* to that component. | ||
* Renders our `KeyboardAvoiderIos` on iOS, `View` on Android. | ||
*/ | ||
export default class KeyboardAvoider extends PureComponent<Props> { | ||
render(): Node { | ||
const { behavior, children, style, keyboardVerticalOffset } = this.props; | ||
const { children, style } = this.props; | ||
|
||
if (Platform.OS === 'android') { | ||
return <View style={style}>{children}</View>; | ||
} | ||
|
||
return ( | ||
<KeyboardAvoidingView | ||
behavior={behavior} | ||
// See comment on this prop in the jsdoc. | ||
keyboardVerticalOffset={keyboardVerticalOffset} | ||
style={style} | ||
> | ||
{children} | ||
</KeyboardAvoidingView> | ||
); | ||
return <KeyboardAvoiderIos style={style}>{children}</KeyboardAvoiderIos>; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
/* @flow strict-local */ | ||
import React, { useRef, useCallback, useState, useEffect } from 'react'; | ||
import type { Node } from 'react'; | ||
import { View, Keyboard, useWindowDimensions, LayoutAnimation, Platform } from 'react-native'; | ||
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; | ||
import type { IOSKeyboardEvent } from 'react-native/Libraries/Components/Keyboard/Keyboard'; | ||
import invariant from 'invariant'; | ||
|
||
type Props = $ReadOnly<{| | ||
children: Node, | ||
style?: ViewStyleProp, | ||
contentContainerStyle?: ViewStyleProp, | ||
|}>; | ||
|
||
type Measurements = null | $ReadOnly<{| x: number, y: number, width: number, height: number |}>; | ||
|
||
/** | ||
* The near-current value of viewRef.measureInWindow; reacts to layout changes | ||
* | ||
* The returned `onLayout` function must be supplied to the View; see | ||
* https://reactnative.dev/docs/view#onlayout. | ||
* | ||
* See also the doc for viewRef.measureInWindow: | ||
* https://reactnative.dev/docs/0.64/direct-manipulation#measureinwindowcallback | ||
* | ||
* "Near-current" because viewRef.measureInWindow is an asynchronous query; | ||
* experimentally, it seems to take 10-50ms. | ||
* | ||
* The X and Y coordinates passed to the `onLayout` callback are relative to | ||
* the parent view. `measureInWindow` gives you coordinates relative to the | ||
* entire window, which is often more useful -- but RN doesn't give us a | ||
* direct hook into when `measureInWindow`'s result changes. This is meant | ||
* to address that lack. | ||
*/ | ||
function useViewMeasurementsInWindow(viewRef) { | ||
const counter = useRef<number>(0); | ||
const [measurements, setMeasurements] = useState<Measurements>(null); | ||
|
||
const onLayout = useCallback( | ||
_ => { | ||
// Ignore the data passed to the callback; we just want to make sure | ||
// `measureInWindow` runs on each layout change. | ||
|
||
counter.current++; | ||
const counterValueNow = counter.current; | ||
|
||
if (!viewRef.current) { | ||
return; | ||
} | ||
|
||
viewRef.current.measureInWindow((x, y, width, height) => { | ||
// `measureInWindow` is asynchronous. If another `measureInWindow` | ||
// started while we were waiting for this one, don't commit the | ||
// result. Otherwise, do. | ||
if (counterValueNow === counter.current) { | ||
setMeasurements({ x, y, width, height }); | ||
counter.current %= Number.MAX_SAFE_INTEGER; | ||
} | ||
}); | ||
}, | ||
[viewRef], | ||
); | ||
|
||
return { | ||
onLayout, | ||
measurements, | ||
}; | ||
} | ||
|
||
/** | ||
* Like RN's `KeyboardAvoidingView`, but with some assumptions/improvements. | ||
* | ||
* Unlike `KeyboardAvoidingView`, we assume: | ||
* - We're on iOS | ||
* - The offset is done with padding (like `behavior="padding"`) | ||
* | ||
* We make these improvements (as of RN v0.64): | ||
* - No fiddly `keyboardVerticalOffset` that no one understands (see | ||
* our 70eca0716) | ||
* - It works even when the user has enabled "Prefer Cross-Fade Transitions" | ||
* (UIAccessibilityPrefersCrossFadeTransitions); see | ||
* https://github.com/facebook/react-native/issues/29974 | ||
* - We hope to make it happier code (easier to read, debug, maintain, etc.) | ||
*/ | ||
// We rely on https://reactnative.dev/docs/keyboard. `KeyboardAvoidingView` | ||
// uses that internally too, and it's a public API. Hopefully that gives us | ||
// some assurance about the stability of our component as RN evolves. | ||
// TODO: Could write some tests for the Hooks logic; see | ||
// `useHasStayedTrueForMs` for testing a hook, and for another possible | ||
// approach, see | ||
// https://github.com/zulip/zulip-mobile/pull/4997#issuecomment-916457062. | ||
// However, we rely heavily on the behavior of | ||
// https://reactnative.dev/docs/keyboard, and that wouldn't be naturally | ||
// covered no matter how we test this component. | ||
export default function KeyboardAvoiderIos(props: Props): Node { | ||
const { children } = props; | ||
|
||
// The most recent IOSKeyboardEvent, if any since mount. | ||
const [keyboardEvent, setKeyboardEvent] = useState(null); | ||
useEffect(() => { | ||
invariant(Platform.OS === 'ios', 'KeyboardAvoiderIos expected to be on iOS'); | ||
|
||
const sub = Keyboard.addListener('keyboardWillChangeFrame', (e: IOSKeyboardEvent) => { | ||
setKeyboardEvent(e); | ||
}); | ||
return () => sub.remove(); | ||
}, []); | ||
|
||
const viewRef = useRef(); | ||
const { onLayout, measurements: viewMeasurementsInWindow } = useViewMeasurementsInWindow(viewRef); | ||
const { height: windowHeight } = useWindowDimensions(); | ||
const [paddingBottom, setPaddingBottom] = useState(0); | ||
useEffect(() => { | ||
if (!viewMeasurementsInWindow || !keyboardEvent) { | ||
return; | ||
} | ||
|
||
const { | ||
y: viewYInWindow, | ||
height: viewHeight, | ||
// ignore view x and width | ||
} = viewMeasurementsInWindow; | ||
const { | ||
endCoordinates: { | ||
height: endKeyboardHeight, | ||
screenY: endKeyboardY, | ||
// Ignore keyboard end x and width | ||
}, | ||
easing, | ||
duration, | ||
// ignore startCoordinates and isEventFromThisApp | ||
// TODO: Only proceed if isEventFromThisApp is true, right? | ||
} = keyboardEvent; | ||
|
||
setPaddingBottom(currentValue => { | ||
const newValue = | ||
// In most environments, `endKeyboardHeight` is some positive value, | ||
// and `endKeyboardY` goes between `windowHeight` (on closing | ||
// keyboard) and something less than `windowHeight` (on opening). | ||
// | ||
// If UIAccessibilityPrefersCrossFadeTransitions, then on closing | ||
// the keyboard, *everything* in `endCoordinates` will be zero, | ||
// including `endKeyboardHeight`. As far as I can tell from reading | ||
// code, those `endCoordinates` are pretty much passed straight | ||
// through from Apple. See facebook/react-native#29974. | ||
// | ||
// We handle both cases with this expression. | ||
endKeyboardY < windowHeight && endKeyboardHeight > 0 | ||
? viewYInWindow + viewHeight - endKeyboardY | ||
: 0; | ||
|
||
if (currentValue === newValue) { | ||
return currentValue; | ||
} | ||
|
||
LayoutAnimation.configureNext({ | ||
duration: Math.max(10, duration), // LayoutAnimation errors on <10ms | ||
update: { | ||
duration: Math.max(10, duration), // LayoutAnimation errors on <10ms | ||
type: LayoutAnimation.Types[easing] || 'keyboard', | ||
}, | ||
}); | ||
|
||
return newValue; | ||
}); | ||
}, [keyboardEvent, viewMeasurementsInWindow, windowHeight]); | ||
|
||
return ( | ||
<View style={[props.style, { paddingBottom }]} ref={viewRef} onLayout={onLayout}> | ||
{children} | ||
</View> | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters