Skip to content

Fix Android MVCP anchor selection when z-index reorders children #57959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hsource wants to merge 1 commit into
base: main
Choose a base branch
from
+182 −114
Open

Fix Android MVCP anchor selection when z-index reorders children #57959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions ...roid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ internal class MaintainVisibleScrollPositionHelper<ScrollViewT>(
return
}
isListening = false
firstVisibleViewRef = null
uIManager.removeUIManagerEventListener(this)
}

Expand Down Expand Up @@ -125,21 +126,34 @@ internal class MaintainVisibleScrollPositionHelper<ScrollViewT>(
val contentView = contentView ?: return

val currentScroll = if (horizontal) scrollView.scrollX else scrollView.scrollY
var firstVisibleView: View? = null
// We cannot assume that the views will be in position order because of things like z-index
// which will change the order of views in their parent. This means we need to iterate through
// the full children array and find the view with the smallest position that is bigger than
// the scroll position.
var firstVisibleViewPosition = Float.MAX_VALUE
for (i in config.minIndexForVisible until contentView.childCount) {
val child = contentView.getChildAt(i)

// Compute the position of the end of the child
val position = if (horizontal) child.x + child.width else child.y + child.height

// If the child is partially visible or this is the last child, select it as the anchor.
if (position > currentScroll || i == contentView.childCount - 1) {
firstVisibleViewRef = WeakReference(child)
val frame = Rect()
child.getHitRect(frame)
prevFirstVisibleFrame = frame
break
if ((position > currentScroll && position < firstVisibleViewPosition) ||
(firstVisibleView == null && i == contentView.childCount - 1)) {
firstVisibleView = child
firstVisibleViewPosition = position
}
}

if (firstVisibleView == null) {
return
}

firstVisibleViewRef = WeakReference(firstVisibleView)
val frame = Rect()
firstVisibleView.getHitRect(frame)
prevFirstVisibleFrame = frame
}

// UIManagerListener
Expand Down
270 changes: 162 additions & 108 deletions packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import RNTesterText from '../../components/RNTesterText';
import ScrollViewPressableStickyHeaderExample from './ScrollViewPressableStickyHeaderExample';
import nullthrows from 'nullthrows';
import * as React from 'react';
import {cloneElement, useCallback, useRef, useState} from 'react';
import {useCallback, useRef, useState} from 'react';
import {
Platform,
RefreshControl,
Expand Down Expand Up @@ -62,114 +62,153 @@ class EnableDisableList extends React.Component<{}, {scrollEnabled: boolean}> {
}

let AppendingListItemCount = 6;
class AppendingList extends React.Component<
{},
{items: Array<ExactReactElement_DEPRECATED<Class<Item>>>},
> {
state: {items: Array<ExactReactElement_DEPRECATED<Class<Item>>>} = {
items: [...Array(AppendingListItemCount)].map((_, ii) => (
<Item msg={`Item ${ii}`} />
)),
};
render(): React.Node {
return (
<View>
<ScrollView
automaticallyAdjustContentInsets={false}
maintainVisibleContentPosition={{
minIndexForVisible: 0,
autoscrollToTopThreshold: 10,

type ItemInfo = {
id: number,
paddingTop?: number,
paddingBottom?: number,
};

function AppendingList(): React.Node {

@hsource hsource Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was made because cloneElement seemed to remove the zIndex, so we instead render items without cloneElement.

While I was at it, I also changed this to a function component.

const [changeAtId, setChangeAtId] = useState('1');
const [items, setItems] = useState<Array<ItemInfo>>(() =>
[...Array(AppendingListItemCount)].map((_, ii) => ({
id: ii,
})),
);

const renderItem = (item: ItemInfo, horizontal: boolean) => (
<Item
key={item.id}
msg={`Item ${item.id}`}
// When changing an item's height, its top position should stay fixed
// rather than its bottom. This used to not be the case with negative
// zIndex.
zIndex={-item.id}
style={
horizontal
? {
paddingLeft: item.paddingTop,
paddingRight: item.paddingBottom,
}
: {
paddingTop: item.paddingTop,
paddingBottom: item.paddingBottom,
}
}
/>
);

return (
<View>
<ScrollView
automaticallyAdjustContentInsets={false}
maintainVisibleContentPosition={{
minIndexForVisible: 0,
autoscrollToTopThreshold: 10,
}}
nestedScrollEnabled
style={styles.scrollView}>
{items.map(item => renderItem(item, false))}
</ScrollView>
<ScrollView
horizontal={true}
automaticallyAdjustContentInsets={false}
maintainVisibleContentPosition={{
minIndexForVisible: 1,
autoscrollToTopThreshold: 10,
}}
style={[styles.scrollView, styles.horizontalScrollView]}>
{items.map(item => renderItem(item, true))}
</ScrollView>
<View style={styles.row}>
<Button
label="Add to top"
onPress={() => {
setItems(prevItems => {
const idx = AppendingListItemCount++;
return [{id: idx, paddingTop: idx * 5}, ...prevItems];
});
}}
nestedScrollEnabled
style={styles.scrollView}>
{this.state.items.map(item =>
// $FlowFixMe[prop-missing] React.Element internal inspection
cloneElement(item, {key: item.props.msg}),
)}
</ScrollView>
<ScrollView
horizontal={true}
automaticallyAdjustContentInsets={false}
maintainVisibleContentPosition={{
minIndexForVisible: 1,
autoscrollToTopThreshold: 10,
/>
<Button
label="Remove top"
onPress={() => {
setItems(prevItems => prevItems.slice(1));
}}
style={[styles.scrollView, styles.horizontalScrollView]}>
{this.state.items.map(item =>
// $FlowFixMe[prop-missing] React.Element internal inspection
cloneElement(item, {key: item.props.msg, style: null}),
)}
</ScrollView>
<View style={styles.row}>
<Button
label="Add to top"
onPress={() => {
this.setState(state => {
const idx = AppendingListItemCount++;
return {
items: [
<Item style={{paddingTop: idx * 5}} msg={`Item ${idx}`} />,
].concat(state.items),
};
});
}}
/>
<Button
label="Remove top"
onPress={() => {
this.setState(state => ({
items: state.items.slice(1),
}));
}}
/>
<Button
label="Change height top"
onPress={() => {
this.setState(state => ({
items: [
cloneElement(state.items[0], {
style: {paddingBottom: Math.random() * 40},
}),
].concat(state.items.slice(1)),
}));
}}
/>
</View>
<View style={styles.row}>
<Button
label="Add to end"
onPress={() => {
this.setState(state => ({
items: state.items.concat(
<Item msg={`Item ${AppendingListItemCount++}`} />,
),
}));
}}
/>
<Button
label="Remove end"
onPress={() => {
this.setState(state => ({
items: state.items.slice(0, -1),
}));
}}
/>
<Button
label="Change height end"
onPress={() => {
this.setState(state => ({
items: state.items.slice(0, -1).concat(
cloneElement(state.items[state.items.length - 1], {
style: {paddingBottom: Math.random() * 40},
}),
),
}));
}}
/>
</View>
/>
<Button
label="Change height top"
onPress={() => {
setItems(prevItems => {
if (prevItems.length === 0) {
return prevItems;
}
const [first, ...rest] = prevItems;
return [{...first, paddingBottom: Math.random() * 40}, ...rest];
});
}}
/>
</View>
);
}
<View style={styles.row}>
<Button
label="Add to end"
onPress={() => {
setItems(prevItems => {
const idx = AppendingListItemCount++;
return [...prevItems, {id: idx}];
});
}}
/>
<Button
label="Remove end"
onPress={() => {
setItems(prevItems => prevItems.slice(0, -1));
}}
/>
<Button
label="Change height end"
onPress={() => {
setItems(prevItems => {
if (prevItems.length === 0) {
return prevItems;
}
const last = prevItems[prevItems.length - 1];
return [
...prevItems.slice(0, -1),
{...last, paddingBottom: Math.random() * 40},
];
});
}}
/>
</View>
<View style={styles.row}>
<TextInput
keyboardType="number-pad"
onChangeText={setChangeAtId}
placeholder="Id"
style={styles.indexInput}
value={changeAtId}
/>
<Button
label="Change height at id"
onPress={() => {
const id = parseInt(changeAtId, 10);
if (Number.isNaN(id)) {
return;
}
setItems(prevItems =>
prevItems.map(item =>
item.id === id
? {...item, paddingBottom: Math.random() * 40}
: item,
),
);
}}
/>
</View>
</View>
);
}

function CenterContentList(): React.Node {
Expand Down Expand Up @@ -436,7 +475,9 @@ const examples: Array<RNTesterModuleExample> = [
title: '<ScrollView> smooth bi-directional content loading\n',
description:
'The `maintainVisibleContentPosition` prop allows insertions to either end of the content ' +
'without causing the visible content to jump. Re-ordering is not supported.',
'without causing the visible content to jump. Re-ordering is not supported. Items use ' +
'inverted z-index values so Fabric may reorder native children; the anchor should still ' +
'be the topmost visible item, not whichever child happens to appear first in the hierarchy.',
render() {
return <AppendingList />;
},
Expand Down Expand Up @@ -1477,10 +1518,12 @@ function ChildrenWithTouchEventsOverflowingContainerHorizontal() {
class Item extends React.PureComponent<{
msg?: string,
style?: ViewStyleProp,
zIndex?: number,
}> {
render(): $FlowFixMe {
return (
<View style={[styles.item, this.props.style]}>
<View
style={[styles.item, this.props.style, {zIndex: this.props.zIndex}]}>
<Text>{this.props.msg}</Text>
</View>
);
Expand Down Expand Up @@ -1537,6 +1580,17 @@ const styles = StyleSheet.create({
flexDirection: 'row',
justifyContent: 'space-around',
},
indexInput: {
alignSelf: 'center',
backgroundColor: '#ffffff',
borderColor: '#cccccc',
borderRadius: 3,
borderWidth: 1,
flex: 1,
margin: 5,
padding: 5,
textAlign: 'center',
},
item: {
margin: 5,
padding: 5,
Expand Down
Loading