Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf4ab59b

Browse files
committed
Use react lifecycle to manage scroll
This fixes a issue where scroll restoration was not working as it was called beforereact renders the restored page. In this change we move the restoration to`useLayoutEffect`.In reviewing the fix, we noticed that `setActivePage` was not being dispatchedin `navigateTo`, and relied on `onHistoryChange` to make the correct changes.We refactor to move the explicit nav logic from `onHistoryChange` and back to`navigateTo`.
1 parent0380ed3 commitf4ab59b

File tree

2 files changed

+87
-79
lines changed

2 files changed

+87
-79
lines changed

‎superglue/lib/components/Navigation.tsx‎

Lines changed: 51 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
importReact,{
22
createContext,
33
useEffect,
4+
useLayoutEffect,
45
forwardRef,
56
useImperativeHandle,
67
ForwardedRef,
78
}from'react'
89
import{urlToPageKey,pathWithoutBZParams}from'../utils'
9-
import{removePage,historyChange,setActivePage}from'../actions'
10+
import{removePage,setActivePage}from'../actions'
1011
import{
1112
HistoryState,
1213
RootState,
@@ -15,7 +16,6 @@ import {
1516
NavigationProviderProps,
1617
AllPages,
1718
SuperglueState,
18-
PageKey,
1919
}from'../types'
2020
import{Update}from'history'
2121
import{useDispatch,useSelector,useStore}from'react-redux'
@@ -53,12 +53,23 @@ const NavigationProvider = forwardRef(function NavigationProvider(
5353
constsuperglue=useSelector<RootState,SuperglueState>(
5454
(state)=>state.superglue
5555
)
56+
constcurrentPageKey=useSelector<RootState,string>(
57+
(state)=>state.superglue.currentPageKey
58+
)
5659
conststore=useStore()
5760

5861
useEffect(()=>{
5962
returnhistory.listen(onHistoryChange)
6063
},[])
6164

65+
useLayoutEffect(()=>{
66+
conststate=history.location.stateasHistoryState
67+
if(state&&'superglue'instate){
68+
const{ posX, posY}=state
69+
setWindowScroll(posX,posY)
70+
}
71+
},[currentPageKey])
72+
6273
useImperativeHandle(
6374
ref,
6475
()=>{
@@ -69,36 +80,14 @@ const NavigationProvider = forwardRef(function NavigationProvider(
6980
[]
7081
)
7182

72-
constvisitAndRestore=(pageKey:PageKey,posX:number,posY:number)=>{
73-
// When the application visit gets called with revisit: true
74-
// - In cases where the response was not redirected, the calculated
75-
// navigationAction is set to 'none' (meaning `navigateTo` immediately returned `false`)
76-
// and so we have restore scroll and the set the active page
77-
// - In cases where the response was redirected, the calculated
78-
// navigationAction is set to 'replace', and is handled gracefully by navigateTo,
79-
// before this method gets called.
80-
// That's why we're only concerned with the first case, but we gracefully warn
81-
// if the application visit did not return the meta object like the dev was supposed to.
82-
returnvisit(pageKey,{revisit:true}).then((meta)=>{
83-
if(meta){
84-
if(meta.navigationAction==='none'){
85-
dispatch(setActivePage({ pageKey}))
86-
setWindowScroll(posX,posY)
87-
}
88-
}else{
89-
console.warn(
90-
`scoll restoration was skipped. Your visit's then funtion
91-
should return the meta object it recieved if you want your
92-
application to restore the page's previous scroll.`
93-
)
94-
}
95-
})
96-
}
97-
9883
constonHistoryChange=({ location, action}:Update):void=>{
9984
conststate=location.stateasHistoryState
10085

101-
if(!state&&location.hash!==''&&action==='POP'){
86+
if(action!=='POP'){
87+
return
88+
}
89+
90+
if(!state&&location.hash!==''){
10291
constnextPageKey=urlToPageKey(location.pathname+location.search)
10392
constcontainsKey=!!pages[nextPageKey]
10493
if(containsKey){
@@ -119,17 +108,8 @@ const NavigationProvider = forwardRef(function NavigationProvider(
119108
}
120109

121110
if(state&&'superglue'instate){
122-
dispatch(
123-
historyChange({
124-
pageKey:state.pageKey,
125-
})
126-
)
127-
128-
if(action!=='POP'){
129-
return
130-
}
131-
132-
const{ pageKey, posX, posY}=state
111+
const{ pageKey}=state
112+
constprevPageKey=store.getState().superglue.currentPageKey
133113
constcontainsKey=!!pages[pageKey]
134114

135115
if(containsKey){
@@ -138,19 +118,38 @@ const NavigationProvider = forwardRef(function NavigationProvider(
138118
switch(restoreStrategy){
139119
case'fromCacheOnly':
140120
dispatch(setActivePage({ pageKey}))
141-
setWindowScroll(posX,posY)
142121
break
143122
case'fromCacheAndRevisitInBackground':
144123
dispatch(setActivePage({ pageKey}))
145-
setWindowScroll(posX,posY)
146124
visit(pageKey,{revisit:true})
147125
break
148126
case'revisitOnly':
149127
default:
150-
visitAndRestore(pageKey,posX,posY)
128+
visit(pageKey,{revisit:true}).then(()=>{
129+
constnoNav=
130+
prevPageKey===store.getState().superglue.currentPageKey
131+
if(noNav){
132+
// When "POP'ed", revisiting (using revisit: true) a page can result in
133+
// a redirect, or a render of the same page.
134+
//
135+
// When its a redirect, calculateNavAction will correctly set the
136+
// navigationAction to `replace` this is the noop scenario.
137+
//
138+
// When its the same page, navigationAction is set to `none` and
139+
// no navigation took place. In that case, we have to set the
140+
// activePage otherwise the user is stuck on the original page.
141+
dispatch(setActivePage({ pageKey}))
142+
}
143+
})
151144
}
152145
}else{
153-
visitAndRestore(pageKey,posX,posY)
146+
visit(pageKey,{revisit:true}).then(()=>{
147+
constnoNav=
148+
prevPageKey===store.getState().superglue.currentPageKey
149+
if(noNav){
150+
dispatch(setActivePage({ pageKey}))
151+
}
152+
})
154153
}
155154
}
156155
}
@@ -175,7 +174,6 @@ const NavigationProvider = forwardRef(function NavigationProvider(
175174
if(hasPage){
176175
constlocation=history.location
177176
conststate=location.stateasHistoryState
178-
constprevPageKey=state.pageKey
179177
consthistoryArgs=[
180178
path,
181179
{
@@ -196,26 +194,24 @@ const NavigationProvider = forwardRef(function NavigationProvider(
196194
},
197195
{
198196
...state,
199-
posY:window.pageYOffset,
200-
posX:window.pageXOffset,
197+
posY:window.scrollY,
198+
posX:window.scrollX,
201199
}
202200
)
203201
}
204202

205203
history.push(...historyArgs)
204+
dispatch(setActivePage({pageKey:nextPageKey}))
206205
}
207206

208207
if(action==='replace'){
209208
history.replace(...historyArgs)
210-
}
211209

212-
setActivePage({pageKey:nextPageKey})
213-
setWindowScroll(0,0)
214-
215-
if(action==='replace'&&prevPageKey&&prevPageKey!==nextPageKey){
216-
dispatch(removePage({pageKey:prevPageKey}))
210+
if(currentPageKey!==nextPageKey){
211+
dispatch(setActivePage({pageKey:nextPageKey}))
212+
dispatch(removePage({pageKey:currentPageKey}))
213+
}
217214
}
218-
219215
returntrue
220216
}else{
221217
console.warn(
@@ -228,7 +224,7 @@ const NavigationProvider = forwardRef(function NavigationProvider(
228224
}
229225
}
230226

231-
const{currentPageKey,search}=superglue
227+
const{ search}=superglue
232228
const{ componentIdentifier}=pages[currentPageKey]
233229
constComponent=mapping[componentIdentifier]
234230

‎superglue/spec/lib/NavComponent.spec.jsx‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import userEvent from '@testing-library/user-event'
88
import{NavigationContext}from'../../lib/components/Navigation'
99
import{configureStore}from'@reduxjs/toolkit'
1010
import{rootReducer}from'../../lib'
11+
import{setActivePage}from'../../lib/actions'
1112

1213
constbuildStore=(preloadedState)=>{
1314
letresultsReducer=(state=[],action)=>{
@@ -175,13 +176,7 @@ describe('Nav', () => {
175176

176177
constexpectedActions=[
177178
{
178-
type:'@@superglue/HISTORY_CHANGE',
179-
payload:{
180-
pageKey:'/home',
181-
},
182-
},
183-
{
184-
type:'@@superglue/HISTORY_CHANGE',
179+
type:'@@superglue/SET_ACTIVE_PAGE',
185180
payload:{
186181
pageKey:'/about',
187182
},
@@ -362,7 +357,7 @@ describe('Nav', () => {
362357

363358
describe('history pop',()=>{
364359
describe('when the previous page was set to "revisitOnly"',()=>{
365-
it('revisits the page and scrolls when finished',()=>{
360+
it('revisits the page and scrolls when finished',async()=>{
366361
consthistory=createMemoryHistory({})
367362
history.replace('/home',{
368363
superglue:true,
@@ -402,9 +397,7 @@ describe('Nav', () => {
402397
constfakeVisit=vi.fn((...args)=>{
403398
return{
404399
then:vi.fn((fn)=>{
405-
expect(scrollTo).not.toHaveBeenCalled()
406400
fn({ navigationAction})
407-
expect(scrollTo).toHaveBeenCalledWith(5,5)
408401
}),
409402
}
410403
})
@@ -421,11 +414,19 @@ describe('Nav', () => {
421414
</Provider>
422415
)
423416

417+
expect(scrollTo).toHaveBeenCalledWith(10,10)
424418
history.back()
419+
expect(scrollTo).not.toHaveBeenCalledWith(5,5)
425420
expect(fakeVisit).toHaveBeenCalledWith('/home',{revisit:true})
421+
422+
awaitexpect.poll(()=>scrollTo.toHaveBeenCalledWith(5,5))
426423
})
427424

428-
it('revisits the page and skips scroll when redirected (navigationAction is not "none")',()=>{
425+
it('revisits the page and when redirected replaces with the new page',async()=>{
426+
constLogin=()=>{
427+
return<h1>Login Page</h1>
428+
}
429+
429430
consthistory=createMemoryHistory({})
430431
history.replace('/home',{
431432
superglue:true,
@@ -451,6 +452,10 @@ describe('Nav', () => {
451452
componentIdentifier:'about',
452453
restoreStrategy:'revisitOnly',
453454
},
455+
'/login':{
456+
componentIdentifier:'login',
457+
restoreStrategy:'fromCacheOnly',
458+
},
454459
},
455460
superglue:{
456461
csrfToken:'abc',
@@ -460,39 +465,45 @@ describe('Nav', () => {
460465
constscrollTo=vi
461466
.spyOn(window,'scrollTo')
462467
.mockImplementation(()=>{})
463-
constnavigationAction='push'
468+
constnavigationAction='replace'
464469

465470
constfakeVisit=vi.fn((...args)=>{
466471
return{
467472
then:vi.fn((fn)=>{
468-
// expect(scrollTo).not.toHaveBeenCalled()
473+
store.dispatch(setActivePage({pageKey:'/login'}))
469474
fn({ navigationAction})
470-
expect(scrollTo).not.toHaveBeenCalled()
471475
}),
472476
}
473477
})
474478

475-
// scroll to 0 0
476-
477479
render(
478480
<Providerstore={store}>
479481
<NavigationProvider
480482
store={store}
481483
visit={fakeVisit}
482-
mapping={{home:Home,about:About}}
484+
mapping={{home:Home,about:About,login:Login}}
483485
initialPageKey={'/home'}
484486
history={history}
485487
/>
486488
</Provider>
487489
)
488490

491+
expect(screen.getByRole('heading')).toHaveTextContent('About Page')
492+
expect(screen.getByRole('heading')).not.toHaveTextContent('Login Page')
493+
489494
history.back()
490-
expect(fakeVisit).toHaveBeenCalledWith('/home',{revisit:true})
495+
496+
awaitexpect.poll(()=>
497+
screen.getByRole('heading').not.toHaveTextContent('About Page')
498+
)
499+
awaitexpect.poll(()=>
500+
screen.getByRole('heading').toHaveTextContent('Login Page')
501+
)
491502
})
492503
})
493504

494505
describe('when the previous page was set to "fromCacheOnly"',()=>{
495-
it('restores without visiting and scrolls',()=>{
506+
it('restores without visiting and scrolls',async()=>{
496507
consthistory=createMemoryHistory({})
497508
history.replace('/home',{
498509
superglue:true,
@@ -541,13 +552,14 @@ describe('Nav', () => {
541552
)
542553

543554
history.back()
544-
expect(scrollTo).toHaveBeenCalledWith(5,5)
545555
expect(fakeVisit).not.toHaveBeenCalled()
556+
expect(scrollTo).not.toHaveBeenCalledWith(5,5)
557+
awaitexpect.poll(()=>scrollTo.toHaveBeenCalledWith(5,5))
546558
})
547559
})
548560

549561
describe('and the previous page was set to "fromCacheAndRevisitInBackground"',()=>{
550-
it('restores, scrolls, and revisits the page in the background',()=>{
562+
it('restores, scrolls, and revisits the page in the background',async()=>{
551563
consthistory=createMemoryHistory({})
552564
history.replace('/home',{
553565
superglue:true,
@@ -582,9 +594,7 @@ describe('Nav', () => {
582594
.spyOn(window,'scrollTo')
583595
.mockImplementation(()=>{})
584596

585-
constfakeVisit=vi.fn((...args)=>{
586-
expect(scrollTo).toHaveBeenCalledWith(5,5)
587-
})
597+
constfakeVisit=vi.fn((...args)=>{})
588598

589599
render(
590600
<Providerstore={store}>
@@ -600,6 +610,8 @@ describe('Nav', () => {
600610

601611
history.back()
602612
expect(fakeVisit).toHaveBeenCalledWith('/home',{revisit:true})
613+
expect(scrollTo).not.toHaveBeenCalledWith(5,5)
614+
awaitexpect.poll(()=>scrollTo.toHaveBeenCalledWith(5,5))
603615
})
604616
})
605617
})

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp