Skip to content

Commit 5328a66

Browse files
committed
fix(app): preserve prior scrollRestoration value across mount/unmount
Address review feedback on the scoped scrollRestoration cleanup: hard-setting 'auto' on unmount could clobber a non-default prior value set elsewhere. Capture history.scrollRestoration on mount and restore that exact value on unmount in both HomePage and PlotsPage. Tests now cover both prior-state cases ('auto' and 'manual').
1 parent 41e1e5a commit 5328a66

4 files changed

Lines changed: 28 additions & 8 deletions

File tree

app/src/pages/HomePage.test.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,20 @@ describe('HomePage', () => {
124124
history.scrollRestoration = original;
125125
});
126126

127-
it("sets scrollRestoration to 'manual' on mount and restores 'auto' on unmount", () => {
127+
it("sets scrollRestoration to 'manual' on mount and restores the previous value on unmount", () => {
128128
history.scrollRestoration = 'auto';
129129
const { unmount } = render(<HomePage />);
130130
expect(history.scrollRestoration).toBe('manual');
131131
unmount();
132132
expect(history.scrollRestoration).toBe('auto');
133133
});
134+
135+
it('restores a non-default previous value on unmount instead of forcing auto', () => {
136+
history.scrollRestoration = 'manual';
137+
const { unmount } = render(<HomePage />);
138+
expect(history.scrollRestoration).toBe('manual');
139+
unmount();
140+
expect(history.scrollRestoration).toBe('manual');
141+
});
134142
});
135143
});

app/src/pages/HomePage.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@ export function HomePage() {
2929
const { homeStateRef, saveScrollPosition } = useHomeState();
3030

3131
// Disable browser's automatic scroll restoration so we can restore from
32-
// our persisted state (homeStateRef.scrollY) instead. Restore on unmount
33-
// so other routes get native back/forward behavior.
32+
// our persisted state (homeStateRef.scrollY) instead. Capture the prior
33+
// mode and restore it on unmount, so we don't clobber any non-default
34+
// value set elsewhere and other routes get back native behavior.
3435
useEffect(() => {
3536
if (!('scrollRestoration' in history)) return;
37+
const previous = history.scrollRestoration;
3638
history.scrollRestoration = 'manual';
3739
return () => {
38-
history.scrollRestoration = 'auto';
40+
history.scrollRestoration = previous;
3941
};
4042
}, []);
4143

app/src/pages/PlotsPage.test.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,20 @@ describe('PlotsPage', () => {
7676
history.scrollRestoration = original;
7777
});
7878

79-
it("sets scrollRestoration to 'manual' on mount and restores 'auto' on unmount", () => {
79+
it("sets scrollRestoration to 'manual' on mount and restores the previous value on unmount", () => {
8080
history.scrollRestoration = 'auto';
8181
const { unmount } = render(<PlotsPage />);
8282
expect(history.scrollRestoration).toBe('manual');
8383
unmount();
8484
expect(history.scrollRestoration).toBe('auto');
8585
});
86+
87+
it('restores a non-default previous value on unmount instead of forcing auto', () => {
88+
history.scrollRestoration = 'manual';
89+
const { unmount } = render(<PlotsPage />);
90+
expect(history.scrollRestoration).toBe('manual');
91+
unmount();
92+
expect(history.scrollRestoration).toBe('manual');
93+
});
8694
});
8795
});

app/src/pages/PlotsPage.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ export function PlotsPage() {
2222
const { homeStateRef, saveScrollPosition } = useHomeState();
2323

2424
// Disable browser's automatic scroll restoration so we can restore from
25-
// our persisted state (homeStateRef.scrollY) instead. Restore on unmount
26-
// so other routes get native back/forward behavior.
25+
// our persisted state (homeStateRef.scrollY) instead. Capture the prior
26+
// mode and restore it on unmount, so we don't clobber any non-default
27+
// value set elsewhere and other routes get back native behavior.
2728
useEffect(() => {
2829
if (!('scrollRestoration' in history)) return;
30+
const previous = history.scrollRestoration;
2931
history.scrollRestoration = 'manual';
3032
return () => {
31-
history.scrollRestoration = 'auto';
33+
history.scrollRestoration = previous;
3234
};
3335
}, []);
3436

0 commit comments

Comments
 (0)