Skip to content

Commit c6f6ae2

Browse files
committed
Improve back navigation logic in ItemComponent
1 parent d3a5aa8 commit c6f6ae2

7 files changed

Lines changed: 144 additions & 10 deletions

File tree

src/app/core/services/route.service.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,34 @@ export class RouteService {
184184
});
185185
}
186186

187+
/**
188+
* Store a URL in session storage for later retrieval
189+
* Generic method that can be used by any component
190+
* @param key The session storage key
191+
* @param url The URL to store
192+
*/
193+
public storeUrlInSession(key: string, url: string): void {
194+
if (typeof window !== 'undefined' && hasValue(window.sessionStorage)) {
195+
// Only write if the value is different to avoid unnecessary writes
196+
const currentValue = window.sessionStorage.getItem(key);
197+
if (currentValue !== url) {
198+
window.sessionStorage.setItem(key, url);
199+
}
200+
}
201+
}
202+
203+
/**
204+
* Retrieve a URL from session storage
205+
* Generic method that can be used by any component
206+
* @param key The session storage key
207+
*/
208+
public getUrlFromSession(key: string): string | null {
209+
if (typeof window !== 'undefined' && hasValue(window.sessionStorage)) {
210+
return window.sessionStorage.getItem(key);
211+
}
212+
return null;
213+
}
214+
187215
private getRouteParams(): Observable<Params> {
188216
let active = this.route;
189217
while (active.firstChild) {

src/app/core/testing/route-service.stub.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ export const routeServiceStub: any = {
4141
getPreviousUrl: () => {
4242
return of('/home');
4343
},
44+
// Added generic session helpers used by ItemComponent
45+
storeUrlInSession: (key: string, url: string) => {
46+
// no-op for tests
47+
},
48+
getUrlFromSession: (key: string): string | null => {
49+
return null;
50+
},
51+
clearUrlFromSession: (key: string) => {
52+
// no-op for tests
53+
},
4454
setParameter: (key: any, value: any) => {
4555
return;
4656
},

src/app/item-page/simple/item-types/dataset/dataset.component.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ describe('DatasetComponent', () => {
213213
getPreviousUrl(): Observable<string> {
214214
return of('/search?query=test%20query&fakeParam=true');
215215
},
216+
storeUrlInSession(_key: string, _url: string): void {
217+
// no-op
218+
},
219+
getUrlFromSession(_key: string): string | null {
220+
return null;
221+
},
216222
};
217223
beforeEach(waitForAsync(() => {
218224
const iiifEnabledMap: MetadataMap = {
@@ -243,6 +249,12 @@ describe('DatasetComponent', () => {
243249
getPreviousUrl(): Observable<string> {
244250
return of('/item');
245251
},
252+
storeUrlInSession(_key: string, _url: string): void {
253+
// no-op
254+
},
255+
getUrlFromSession(_key: string): string | null {
256+
return null;
257+
},
246258
};
247259
beforeEach(waitForAsync(() => {
248260
const iiifEnabledMap: MetadataMap = {

src/app/item-page/simple/item-types/publication/publication.component.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,12 @@ describe('PublicationComponent', () => {
222222
getPreviousUrl(): Observable<string> {
223223
return of('/search?query=test%20query&fakeParam=true');
224224
},
225+
storeUrlInSession(key: string, url: string): void {
226+
// no-op
227+
},
228+
getUrlFromSession(key: string): string | null {
229+
return null;
230+
},
225231
};
226232
beforeEach(waitForAsync(() => {
227233
const iiifEnabledMap: MetadataMap = {
@@ -252,6 +258,12 @@ describe('PublicationComponent', () => {
252258
getPreviousUrl(): Observable<string> {
253259
return of('/item');
254260
},
261+
storeUrlInSession(key: string, url: string): void {
262+
// no-op
263+
},
264+
getUrlFromSession(key: string): string | null {
265+
return null;
266+
},
255267
};
256268
beforeEach(waitForAsync(() => {
257269
const iiifEnabledMap: MetadataMap = {

src/app/item-page/simple/item-types/shared/item.component.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ export const mockRouteService = {
104104
getPreviousUrl(): Observable<string> {
105105
return of('');
106106
},
107+
storeUrlInSession(key: string, url: string): void {
108+
// no-op
109+
},
110+
getUrlFromSession(key: string): string | null {
111+
return null;
112+
},
107113
getQueryParameterValue(): Observable<string> {
108114
return of('');
109115
},
@@ -485,6 +491,7 @@ describe('ItemComponent', () => {
485491

486492
const searchUrl = '/search?query=test&spc.page=2';
487493
const browseUrl = '/browse/title?scope=0cc&bbm.page=3';
494+
const homeUrl = '/home';
488495
const recentSubmissionsUrl = '/collections/be7b8430-77a5-4016-91c9-90863e50583a?cp.page=3';
489496

490497
beforeEach(waitForAsync(() => {
@@ -566,6 +573,32 @@ describe('ItemComponent', () => {
566573
expect(val).toBeTrue();
567574
});
568575
});
576+
577+
it('should show back button for home', () => {
578+
spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(of(homeUrl));
579+
comp.ngOnInit();
580+
comp.showBackButton$.subscribe((val) => {
581+
expect(val).toBeTrue();
582+
});
583+
});
584+
585+
it('should prioritize home previous url over session fallback', () => {
586+
const staleSessionUrl = searchUrl;
587+
const getPreviousUrlSpy = spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(of(homeUrl));
588+
const getUrlFromSessionSpy = spyOn(mockRouteService, 'getUrlFromSession').and.returnValue(staleSessionUrl);
589+
const storeUrlInSessionSpy = spyOn(mockRouteService, 'storeUrlInSession');
590+
591+
comp.ngOnInit();
592+
comp.showBackButton$.subscribe((val) => {
593+
expect(val).toBeTrue();
594+
expect(getPreviousUrlSpy).toHaveBeenCalled();
595+
expect(getUrlFromSessionSpy).not.toHaveBeenCalled();
596+
expect(storeUrlInSessionSpy).toHaveBeenCalledWith('item-previous-url', homeUrl);
597+
598+
comp.back();
599+
expect(router.navigateByUrl).toHaveBeenCalledWith(homeUrl);
600+
});
601+
});
569602
});
570603

571604
});

src/app/item-page/simple/item-types/shared/item.component.ts

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,16 @@ export class ItemComponent implements OnInit {
4141
*/
4242
@Input() viewMode: ViewMode;
4343

44+
/**
45+
* Session storage key for storing the previous URL before entering item page
46+
*/
47+
private readonly ITEM_PREVIOUS_URL_SESSION_KEY = 'item-previous-url';
48+
4449
/**
4550
* This regex matches previous routes. The button is shown
4651
* for matching paths and hidden in other cases.
4752
*/
48-
previousRoute = /^(\/search|\/browse|\/collections|\/admin\/search|\/mydspace)/;
53+
previousRoute = /^(\/home|\/search|\/browse|\/collections|\/admin\/search|\/mydspace)/;
4954

5055
/**
5156
* Used to show or hide the back to results button in the view.
@@ -79,6 +84,11 @@ export class ItemComponent implements OnInit {
7984
*/
8085
geospatialItemPageFieldsEnabled = false;
8186

87+
/**
88+
* Stores the previous URL retrieved either from RouteService or sessionStorage
89+
*/
90+
private storedPreviousUrl: string;
91+
8292
/**
8393
* Flag to check whether to use the default relations or the authority based ones
8494
*/
@@ -96,24 +106,34 @@ export class ItemComponent implements OnInit {
96106

97107
/**
98108
* The function used to return to list from the item.
109+
* Uses stored previous URL if available, otherwise falls back to browser history.
99110
*/
100111
back = () => {
101-
this.routeService.getPreviousUrl().pipe(
102-
take(1),
103-
).subscribe(
104-
(url => {
105-
this.router.navigateByUrl(url);
106-
}),
107-
);
112+
this.router.navigateByUrl(this.storedPreviousUrl);
108113
};
109114

110115
ngOnInit(): void {
111-
112116
this.itemPageRoute = getItemPageRoute(this.object);
113117
// hide/show the back button
114118
this.showBackButton$ = this.routeService.getPreviousUrl().pipe(
115-
map((url: string) => this.previousRoute.test(url)),
116119
take(1),
120+
map(url => {
121+
const fromRoute = this.pickAllowedPrevious(url);
122+
123+
if (fromRoute) {
124+
this.routeService.storeUrlInSession(this.ITEM_PREVIOUS_URL_SESSION_KEY, fromRoute);
125+
this.storedPreviousUrl = fromRoute;
126+
return true;
127+
}
128+
129+
const storedUrl = this.routeService.getUrlFromSession(this.ITEM_PREVIOUS_URL_SESSION_KEY);
130+
if (this.pickAllowedPrevious(storedUrl)) {
131+
this.storedPreviousUrl = storedUrl;
132+
return true;
133+
}
134+
135+
return false;
136+
}),
117137
);
118138
// check to see if iiif viewer is required.
119139
this.iiifEnabled = isIiifEnabled(this.object);
@@ -122,4 +142,11 @@ export class ItemComponent implements OnInit {
122142
this.iiifQuery$ = getDSpaceQuery(this.object, this.routeService);
123143
}
124144
}
145+
146+
/**
147+
* Helper to check if a URL is from an allowed previous route and return it, otherwise null
148+
*/
149+
private pickAllowedPrevious(url?: string | null): string | null {
150+
return url && this.previousRoute.test(url) ? url : null;
151+
}
125152
}

src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,12 @@ describe('UntypedItemComponent', () => {
242242
getPreviousUrl(): Observable<string> {
243243
return of('/search?query=test%20query&fakeParam=true');
244244
},
245+
storeUrlInSession(key: string, url: string): void {
246+
// no-op
247+
},
248+
getUrlFromSession(key: string): string | null {
249+
return null;
250+
},
245251
};
246252
beforeEach(waitForAsync(() => {
247253
const iiifEnabledMap: MetadataMap = {
@@ -273,6 +279,12 @@ describe('UntypedItemComponent', () => {
273279
getPreviousUrl(): Observable<string> {
274280
return of('/item');
275281
},
282+
storeUrlInSession(key: string, url: string): void {
283+
// no-op
284+
},
285+
getUrlFromSession(key: string): string | null {
286+
return null;
287+
},
276288
};
277289
beforeEach(waitForAsync(() => {
278290
const iiifEnabledMap: MetadataMap = {

0 commit comments

Comments
 (0)