Fixes for thread composer on Android (#6045)

* Extract function to read contentHeight later

* Remove autoscroll to bottom

We're going to implement this in the UI layer instead.

* Remove worklet from non-worklets to avoid confusion

* Rename and invert hasScrolled* variables

Their naming was too ambiguous (they used to represent "has scrolled _away_ from X"). I inverted them and clarified the naming. No functional changes.

* This should not be necessary

It's already called not just from UI thread. And it only sets shared values, which can be done from either thread.

* Make hasScrolledTo* derived values

It wasn't always correct to derive them manually because reading from .value is stale on JS thread. We could fix that by using the local variables but it makes more conceptualy sense to treat these as derived anyway.

* Reimplement autoscroll-to-bottom in UI layer

Doing it here ensures we also do it when you add an image at the end of the thread. Otherwise it's very confusing where it went.

* Use fancy ScrollView

This seems to fix ScrollView getting stuck after inserting images at the thread end on Android.

* More aggressive scroll-to-bottom

* "Fix" cursor getting stuck on Android

* Revert "Use fancy ScrollView"

This reverts commit 04e34a54e3b75f8a77de5062bff5fe6e76420bbb.
This commit is contained in:
dan
2024-11-01 03:47:53 +00:00
committed by GitHub
parent 7a08d61d88
commit 21b82fa19c
2 changed files with 52 additions and 37 deletions
+52 -33
View File
@@ -22,6 +22,7 @@ import {
// @ts-expect-error no type definition
import ProgressCircle from 'react-native-progress/Circle'
import Animated, {
AnimatedRef,
Easing,
FadeIn,
FadeOut,
@@ -507,25 +508,24 @@ export const ComposePost = ({
useEffect(() => {
if (composerState.mutableNeedsFocusActive) {
composerState.mutableNeedsFocusActive = false
textInput.current?.focus()
// On Android, this risks getting the cursor stuck behind the keyboard.
// Not worth it.
if (!isAndroid) {
textInput.current?.focus()
}
}
}, [composerState])
const {
contentHeight,
scrollHandler,
onScrollViewContentSizeChange,
onScrollViewLayout,
topBarAnimatedStyle,
bottomBarAnimatedStyle,
} = useAnimatedBorders()
useEffect(() => {
if (composerState.mutableNeedsScrollToBottom) {
composerState.mutableNeedsScrollToBottom = false
runOnUI(scrollTo)(scrollViewRef, 0, contentHeight.value, true)
}
}, [composerState, scrollViewRef, contentHeight])
} = useScrollTracker({
scrollViewRef,
stickyBottom: true,
})
const keyboardVerticalOffset = useKeyboardVerticalOffset()
@@ -1213,17 +1213,30 @@ export function useComposerCancelRef() {
return useRef<CancelRef>(null)
}
function useAnimatedBorders() {
function useScrollTracker({
scrollViewRef,
stickyBottom,
}: {
scrollViewRef: AnimatedRef<Animated.ScrollView>
stickyBottom: boolean
}) {
const t = useTheme()
const hasScrolledTop = useSharedValue(0)
const hasScrolledBottom = useSharedValue(0)
const contentOffset = useSharedValue(0)
const scrollViewHeight = useSharedValue(Infinity)
const contentHeight = useSharedValue(0)
/**
* Make sure to run this on the UI thread!
*/
const hasScrolledToTop = useDerivedValue(() =>
withTiming(contentOffset.value === 0 ? 1 : 0),
)
const hasScrolledToBottom = useDerivedValue(() =>
withTiming(
contentHeight.value - contentOffset.value - 5 <= scrollViewHeight.value
? 1
: 0,
),
)
const showHideBottomBorder = useCallback(
({
newContentHeight,
@@ -1235,27 +1248,19 @@ function useAnimatedBorders() {
newScrollViewHeight?: number
}) => {
'worklet'
if (typeof newContentHeight === 'number')
contentHeight.value = Math.floor(newContentHeight)
if (typeof newContentOffset === 'number')
contentOffset.value = Math.floor(newContentOffset)
if (typeof newScrollViewHeight === 'number')
scrollViewHeight.value = Math.floor(newScrollViewHeight)
hasScrolledBottom.value = withTiming(
contentHeight.value - contentOffset.value - 5 > scrollViewHeight.value
? 1
: 0,
)
},
[contentHeight, contentOffset, scrollViewHeight, hasScrolledBottom],
[contentHeight, contentOffset, scrollViewHeight],
)
const scrollHandler = useAnimatedScrollHandler({
onScroll: event => {
'worklet'
hasScrolledTop.value = withTiming(event.contentOffset.y > 0 ? 1 : 0)
showHideBottomBorder({
newContentOffset: event.contentOffset.y,
newContentHeight: event.contentSize.height,
@@ -1266,17 +1271,32 @@ function useAnimatedBorders() {
const onScrollViewContentSizeChange = useCallback(
(_width: number, height: number) => {
'worklet'
if (stickyBottom && height > contentHeight.value) {
const isFairlyCloseToBottom =
contentHeight.value - contentOffset.value - 100 <=
scrollViewHeight.value
if (isFairlyCloseToBottom) {
runOnUI(() => {
scrollTo(scrollViewRef, 0, contentHeight.value, true)
})()
}
}
showHideBottomBorder({
newContentHeight: height,
})
},
[showHideBottomBorder],
[
showHideBottomBorder,
scrollViewRef,
contentHeight,
stickyBottom,
contentOffset,
scrollViewHeight,
],
)
const onScrollViewLayout = useCallback(
(evt: LayoutChangeEvent) => {
'worklet'
showHideBottomBorder({
newScrollViewHeight: evt.nativeEvent.layout.height,
})
@@ -1288,9 +1308,9 @@ function useAnimatedBorders() {
return {
borderBottomWidth: StyleSheet.hairlineWidth,
borderColor: interpolateColor(
hasScrolledTop.value,
hasScrolledToTop.value,
[0, 1],
['transparent', t.atoms.border_contrast_medium.borderColor],
[t.atoms.border_contrast_medium.borderColor, 'transparent'],
),
}
})
@@ -1298,15 +1318,14 @@ function useAnimatedBorders() {
return {
borderTopWidth: StyleSheet.hairlineWidth,
borderColor: interpolateColor(
hasScrolledBottom.value,
hasScrolledToBottom.value,
[0, 1],
['transparent', t.atoms.border_contrast_medium.borderColor],
[t.atoms.border_contrast_medium.borderColor, 'transparent'],
),
}
})
return {
contentHeight,
scrollHandler,
onScrollViewContentSizeChange,
onScrollViewLayout,
-4
View File
@@ -87,7 +87,6 @@ export type ComposerState = {
thread: ThreadDraft
activePostIndex: number
mutableNeedsFocusActive: boolean
mutableNeedsScrollToBottom: boolean
}
export type ComposerAction =
@@ -157,7 +156,6 @@ export function composerReducer(
}
case 'add_post': {
const activePostIndex = state.activePostIndex
const isAtTheEnd = activePostIndex === state.thread.posts.length - 1
const nextPosts = [...state.thread.posts]
nextPosts.splice(activePostIndex + 1, 0, {
id: nanoid(),
@@ -172,7 +170,6 @@ export function composerReducer(
})
return {
...state,
mutableNeedsScrollToBottom: isAtTheEnd,
thread: {
...state.thread,
posts: nextPosts,
@@ -514,7 +511,6 @@ export function createComposerState({
return {
activePostIndex: 0,
mutableNeedsFocusActive: false,
mutableNeedsScrollToBottom: false,
thread: {
posts: [
{