Skip to content

Commit e9e5f13

Browse files
committed
Apply code review feedback
1 parent 6b66e62 commit e9e5f13

File tree

1 file changed

+18
-26
lines changed

1 file changed

+18
-26
lines changed

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3275,7 +3275,7 @@ if (enableFragmentRefsScrollIntoView) {
32753275
getFragmentParentHostFiber(this._fragmentFiber);
32763276
if (targetFiber === null) {
32773277
if (__DEV__) {
3278-
console.error(
3278+
console.warn(
32793279
'You are attempting to scroll a FragmentInstance that has no ' +
32803280
'children, siblings, or parent. No scroll was performed.',
32813281
);
@@ -3292,11 +3292,15 @@ if (enableFragmentRefsScrollIntoView) {
32923292
};
32933293
}
32943294

3295-
function isInstanceScrollable(inst: Instance): 0 | 1 | 2 {
3295+
const CONTAINER_STD = 0;
3296+
const CONTAINER_FIXED = 1;
3297+
const CONTAINER_SCROLL = 2;
3298+
type ScrollableContainerType = 0 | 1 | 2;
3299+
function isInstanceScrollable(inst: Instance): ScrollableContainerType {
32963300
const style = getComputedStyle(inst);
32973301

32983302
if (style.position === 'fixed') {
3299-
return 1;
3303+
return CONTAINER_FIXED;
33003304
}
33013305

33023306
if (
@@ -3307,10 +3311,10 @@ function isInstanceScrollable(inst: Instance): 0 | 1 | 2 {
33073311
style.overflowX === 'auto' ||
33083312
style.overflowX === 'scroll'
33093313
) {
3310-
return 2;
3314+
return CONTAINER_SCROLL;
33113315
}
33123316

3313-
return 0;
3317+
return CONTAINER_STD;
33143318
}
33153319

33163320
function searchDOMUntilCommonAncestor<T>(
@@ -3405,29 +3409,22 @@ function scrollIntoViewByScrollContainer(
34053409
children: Array<Fiber>,
34063410
alignToTop: boolean,
34073411
): void {
3408-
if (children.length === 0) {
3409-
return;
3410-
}
3411-
34123412
// Loop through the children, order dependent on alignToTop
34133413
// Each time we reach a new scroll container, we look back at the last one
34143414
// and scroll the first or last child in that container, depending on alignToTop
34153415
// alignToTop=true means iterate in reverse, scrolling the first child of each container
34163416
// alignToTop=false means iterate in normal order, scrolling the last child of each container
34173417
let prevScrolledInstance = null;
34183418
let prevContainerIsFixed = false;
3419-
let currentGroupEnd = alignToTop ? children.length - 1 : 0;
3420-
34213419
let i = alignToTop ? children.length - 1 : 0;
34223420
// We extend the loop one iteration beyond the actual children to handle the last group
34233421
while (i !== (alignToTop ? -2 : children.length + 1)) {
34243422
const isLastGroup = i < 0 || i >= children.length;
3425-
// 1 = fixed, 2 = scrollable, 0 = neither
3426-
let isNewScrollContainer: null | 0 | 1 | 2 = null;
3423+
let isNewScrollContainer: null | ScrollableContainerType = null;
34273424

34283425
if (isLastGroup) {
34293426
// We're past the end, treat as new scroll container to complete the last group
3430-
isNewScrollContainer = 2;
3427+
isNewScrollContainer = CONTAINER_SCROLL;
34313428
} else {
34323429
const child = children[i];
34333430
const instance = getInstanceFromHostFiber<Instance>(child);
@@ -3438,10 +3435,10 @@ function scrollIntoViewByScrollContainer(
34383435
if (prevInstance.parentNode === instance.parentNode) {
34393436
// If these are DOM siblings, check if either is fixed
34403437
isNewScrollContainer =
3441-
isInstanceScrollable(prevInstance) === 1 ||
3442-
isInstanceScrollable(instance) === 1
3443-
? 1
3444-
: 0;
3438+
isInstanceScrollable(prevInstance) === CONTAINER_FIXED ||
3439+
isInstanceScrollable(instance) === CONTAINER_FIXED
3440+
? CONTAINER_FIXED
3441+
: CONTAINER_STD;
34453442
} else {
34463443
isNewScrollContainer = searchDOMUntilCommonAncestor(
34473444
instance,
@@ -3456,9 +3453,9 @@ function scrollIntoViewByScrollContainer(
34563453
// We found a new scroll container, so scroll the appropriate child from the previous group
34573454
let childToScrollIndex;
34583455
if (alignToTop) {
3459-
childToScrollIndex = isLastGroup ? 0 : currentGroupEnd;
3456+
childToScrollIndex = isLastGroup ? 0 : alignToTop ? i + 1 : i - 1;
34603457
} else {
3461-
childToScrollIndex = currentGroupEnd;
3458+
childToScrollIndex = alignToTop ? i + 1 : i - 1;
34623459
}
34633460

34643461
if (childToScrollIndex >= 0 && childToScrollIndex < children.length) {
@@ -3474,16 +3471,11 @@ function scrollIntoViewByScrollContainer(
34743471
);
34753472
if (didScroll) {
34763473
prevScrolledInstance = instanceToScroll;
3477-
prevContainerIsFixed = isNewScrollContainer === 1;
3474+
prevContainerIsFixed = isNewScrollContainer === CONTAINER_FIXED;
34783475
}
34793476
}
34803477
}
34813478

3482-
if (!isLastGroup) {
3483-
// Start a new group
3484-
currentGroupEnd = i;
3485-
}
3486-
34873479
i += alignToTop ? -1 : 1;
34883480
}
34893481
}

0 commit comments

Comments
 (0)