From 8b78ed3a97549effbc4d541fae1266af49670eab Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 11:11:07 +0200 Subject: [PATCH 01/10] 68067: Fix adding/removing menu sections --- src/app/shared/menu/menu.component.ts | 4 ++-- src/app/shared/menu/menu.reducer.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 1e81fa2dda..142471224d 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -3,7 +3,7 @@ import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from '../../shared/menu/menu.service'; import { MenuID } from '../../shared/menu/initial-menus-state'; import { MenuSection } from '../../shared/menu/menu.reducer'; -import { first, map } from 'rxjs/operators'; +import { distinctUntilChanged, first, map } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; import { hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; @@ -72,7 +72,7 @@ export class MenuComponent implements OnInit { this.menuCollapsed = this.menuService.isMenuCollapsed(this.menuID); this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID); this.menuVisible = this.menuService.isMenuVisible(this.menuID); - this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(first()); + this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(distinctUntilChanged((x, y) => x.length === y.length)); this.sections.subscribe((sections: MenuSection[]) => { sections.forEach((section: MenuSection) => { this.sectionInjectors.set(section.id, this.getSectionDataInjector(section)); diff --git a/src/app/shared/menu/menu.reducer.ts b/src/app/shared/menu/menu.reducer.ts index 078343a990..e5a30770d8 100644 --- a/src/app/shared/menu/menu.reducer.ts +++ b/src/app/shared/menu/menu.reducer.ts @@ -176,7 +176,9 @@ function removeSection(state: MenusState, action: RemoveMenuSectionAction) { const menuState: MenuState = state[action.menuID]; const id = action.id; const newState = removeFromIndex(state, menuState.sections[action.id], action.menuID); - const newMenuState = Object.assign({}, newState[action.menuID]); + const newMenuState = Object.assign({}, newState[action.menuID], { + sections: Object.assign({}, newState[action.menuID].sections) + }); delete newMenuState.sections[id]; return Object.assign({}, newState, { [action.menuID]: newMenuState }); } From 172d0d986b9b5bdd97d1aab7d23256d4cf4e3d05 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 12:08:59 +0200 Subject: [PATCH 02/10] 68067: JSON.stringify to compare menu section changes --- src/app/shared/menu/menu.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 142471224d..29a3e0a30a 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -3,7 +3,7 @@ import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from '../../shared/menu/menu.service'; import { MenuID } from '../../shared/menu/initial-menus-state'; import { MenuSection } from '../../shared/menu/menu.reducer'; -import { distinctUntilChanged, first, map } from 'rxjs/operators'; +import { distinctUntilChanged, first, map, tap } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; import { hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; @@ -72,7 +72,7 @@ export class MenuComponent implements OnInit { this.menuCollapsed = this.menuService.isMenuCollapsed(this.menuID); this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID); this.menuVisible = this.menuService.isMenuVisible(this.menuID); - this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(distinctUntilChanged((x, y) => x.length === y.length)); + this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(distinctUntilChanged((x, y) => JSON.stringify(x) === JSON.stringify(y))); this.sections.subscribe((sections: MenuSection[]) => { sections.forEach((section: MenuSection) => { this.sectionInjectors.set(section.id, this.getSectionDataInjector(section)); From ff0750d0532e420da1891998d403b5efd2061dbd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 14:37:17 +0200 Subject: [PATCH 03/10] 68067: Add menu sections through route data --- .../admin-sidebar/admin-sidebar.component.ts | 8 ++- src/app/navbar/navbar.component.ts | 12 ++-- src/app/shared/menu/menu.actions.ts | 14 +++++ src/app/shared/menu/menu.component.ts | 55 ++++++++++++++++++- src/app/shared/menu/menu.reducer.ts | 6 +- src/app/shared/menu/menu.service.ts | 16 ++++-- 6 files changed, 93 insertions(+), 18 deletions(-) diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index 5d885b918b..d3633c86b2 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -18,6 +18,7 @@ import { TextMenuItemModel } from '../../shared/menu/menu-item/models/text.model import { MenuComponent } from '../../shared/menu/menu.component'; import { MenuService } from '../../shared/menu/menu.service'; import { CSSVariableService } from '../../shared/sass-helper/sass-helper.service'; +import { ActivatedRoute, Router } from '@angular/router'; /** * Component representing the admin sidebar @@ -59,18 +60,19 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { constructor(protected menuService: MenuService, protected injector: Injector, + protected route: ActivatedRoute, + protected router: Router, private variableService: CSSVariableService, private authService: AuthService, private modalService: NgbModal ) { - super(menuService, injector); + super(menuService, injector, route, router); } /** * Set and calculate all initial values of the instance variables */ ngOnInit(): void { - this.createMenu(); super.ngOnInit(); this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); this.authService.isAuthenticated() @@ -93,7 +95,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { /** * Initialize all menu sections and items for this menu */ - private createMenu() { + createMenu() { const menuList = [ /* News */ { diff --git a/src/app/navbar/navbar.component.ts b/src/app/navbar/navbar.component.ts index 6055aac263..99e44ff489 100644 --- a/src/app/navbar/navbar.component.ts +++ b/src/app/navbar/navbar.component.ts @@ -7,6 +7,7 @@ import { TextMenuItemModel } from '../shared/menu/menu-item/models/text.model'; import { LinkMenuItemModel } from '../shared/menu/menu-item/models/link.model'; import { HostWindowService } from '../shared/host-window.service'; import { environment } from '../../environments/environment'; +import { ActivatedRoute, Router } from '@angular/router'; /** * Component representing the public navbar @@ -17,7 +18,7 @@ import { environment } from '../../environments/environment'; templateUrl: './navbar.component.html', animations: [slideMobileNav] }) -export class NavbarComponent extends MenuComponent implements OnInit { +export class NavbarComponent extends MenuComponent { /** * The menu ID of the Navbar is PUBLIC * @type {MenuID.PUBLIC} @@ -26,14 +27,11 @@ export class NavbarComponent extends MenuComponent implements OnInit { constructor(protected menuService: MenuService, protected injector: Injector, + protected route: ActivatedRoute, + protected router: Router, public windowService: HostWindowService ) { - super(menuService, injector); - } - - ngOnInit(): void { - this.createMenu(); - super.ngOnInit(); + super(menuService, injector, route, router); } /** diff --git a/src/app/shared/menu/menu.actions.ts b/src/app/shared/menu/menu.actions.ts index 00275441d6..5b9166641e 100644 --- a/src/app/shared/menu/menu.actions.ts +++ b/src/app/shared/menu/menu.actions.ts @@ -21,6 +21,7 @@ export const MenuActionTypes = { EXPAND_MENU_PREVIEW: type('dspace/menu/EXPAND_MENU_PREVIEW'), ADD_SECTION: type('dspace/menu-section/ADD_SECTION'), REMOVE_SECTION: type('dspace/menu-section/REMOVE_SECTION'), + RESET_SECTIONS: type('dspace/menu-section/RESET_SECTIONS'), SHOW_SECTION: type('dspace/menu-section/SHOW_SECTION'), HIDE_SECTION: type('dspace/menu-section/HIDE_SECTION'), ACTIVATE_SECTION: type('dspace/menu-section/ACTIVATE_SECTION'), @@ -115,6 +116,18 @@ export class ExpandMenuPreviewAction implements Action { } } +/** + * Action used to remove all sections from a certain menu + */ +export class ResetMenuSectionsAction implements Action { + type = MenuActionTypes.RESET_SECTIONS; + menuID: MenuID; + + constructor(menuID: MenuID) { + this.menuID = menuID; + } +} + // MENU SECTION ACTIONS /** * Action used to perform state changes for a section of a certain menu @@ -225,4 +238,5 @@ export type MenuAction = | ToggleActiveMenuSectionAction | CollapseMenuPreviewAction | ExpandMenuPreviewAction + | ResetMenuSectionsAction /* tslint:enable:max-classes-per-file */ diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 29a3e0a30a..4e29ea0ee0 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -3,11 +3,13 @@ import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from '../../shared/menu/menu.service'; import { MenuID } from '../../shared/menu/initial-menus-state'; import { MenuSection } from '../../shared/menu/menu.reducer'; -import { distinctUntilChanged, first, map, tap } from 'rxjs/operators'; +import { distinctUntilChanged, filter, first, map, switchMap, tap } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { hasValue } from '../empty.util'; +import { hasNoValue, hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; +import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; +import { of as observableOf } from 'rxjs/internal/observable/of'; /** * A basic implementation of a MenuComponent @@ -62,13 +64,17 @@ export class MenuComponent implements OnInit { */ private previewTimer; - constructor(protected menuService: MenuService, protected injector: Injector) { + constructor(protected menuService: MenuService, + protected injector: Injector, + protected route: ActivatedRoute, + protected router: Router) { } /** * Sets all instance variables to their initial values */ ngOnInit(): void { + this.initSections(); this.menuCollapsed = this.menuService.isMenuCollapsed(this.menuID); this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID); this.menuVisible = this.menuService.isMenuVisible(this.menuID); @@ -81,6 +87,49 @@ export class MenuComponent implements OnInit { }); } + /** + * Initialize all menu sections and add them to the menu's store + */ + initSections() { + this.router.events.pipe( + filter((e): e is NavigationEnd => e instanceof NavigationEnd), + tap(() => this.menuService.resetSections(this.menuID)), + map(() => this.resolveMenuSections(this.route.root)) + ).subscribe((sections: MenuSection[]) => { + this.createMenu(); + sections.forEach((section: MenuSection) => { + this.menuService.addSection(this.menuID, section); + }); + }); + } + + /** + * Initialize all static menu sections and items for this menu + * These sections will be available on any route. Route specific sections should be defined in the route's data instead. + */ + createMenu() { + // Override this method to create and add a standard set of menu sections that should be available for the current menu type on all routes + } + + /** + * Resolve menu sections defined in the current route data (including parent routes) + * @param route The route to resolve data for + */ + resolveMenuSections(route: ActivatedRoute): MenuSection[] { + const data = route.snapshot.data; + const last: boolean = hasNoValue(route.firstChild); + + if (hasValue(data) && hasValue(data.menu)) { + if (!last) { + return [...data.menu, ...this.resolveMenuSections(route.firstChild)] + } else { + return [...data.menu]; + } + } + + return !last ? this.resolveMenuSections(route.firstChild) : []; + } + /** * Collapse this menu when it's currently expanded, expand it when its currently collapsed * @param {Event} event The user event that triggered this method diff --git a/src/app/shared/menu/menu.reducer.ts b/src/app/shared/menu/menu.reducer.ts index e5a30770d8..85d6847e69 100644 --- a/src/app/shared/menu/menu.reducer.ts +++ b/src/app/shared/menu/menu.reducer.ts @@ -6,7 +6,7 @@ import { MenuAction, MenuActionTypes, MenuSectionAction, - RemoveMenuSectionAction, + RemoveMenuSectionAction, ResetMenuSectionsAction, ShowMenuSectionAction, ToggleActiveMenuSectionAction } from './menu.actions'; @@ -118,6 +118,10 @@ export function menusReducer(state: MenusState = initialMenusState, action: Menu case MenuActionTypes.SHOW_SECTION: { return showSection(state, action as ShowMenuSectionAction); } + case MenuActionTypes.RESET_SECTIONS: { + const newMenuState = Object.assign({}, menuState, { sections: {} }); + return Object.assign({}, state, { [action.menuID]: newMenuState }); + } default: { return state; diff --git a/src/app/shared/menu/menu.service.ts b/src/app/shared/menu/menu.service.ts index 95611daa21..9ff4c2f788 100644 --- a/src/app/shared/menu/menu.service.ts +++ b/src/app/shared/menu/menu.service.ts @@ -14,12 +14,12 @@ import { ExpandMenuAction, ExpandMenuPreviewAction, HideMenuAction, - RemoveMenuSectionAction, + RemoveMenuSectionAction, ResetMenuSectionsAction, ShowMenuAction, ToggleActiveMenuSectionAction, ToggleMenuAction, } from './menu.actions'; -import { hasNoValue, hasValue, isNotEmpty } from '../empty.util'; +import { hasNoValue, hasValue, hasValueOperator, isNotEmpty } from '../empty.util'; export function menuKeySelector(key: string, selector): MemoizedSelector { return createSelector(selector, (state) => { @@ -98,7 +98,7 @@ export class MenuService { switchMap((ids: string[]) => observableCombineLatest(ids.map((id: string) => this.getMenuSection(menuID, id))) ), - map((sections: MenuSection[]) => sections.filter((section: MenuSection) => !mustBeVisible || section.visible)) + map((sections: MenuSection[]) => sections.filter((section: MenuSection) => hasValue(section) && (!mustBeVisible || section.visible))) ); } @@ -147,6 +147,14 @@ export class MenuService { this.store.dispatch(new RemoveMenuSectionAction(menuID, sectionID)); } + /** + * Remove all sections within a menu from the store + * @param {MenuID} menuID The menu from which the sections are to be removed + */ + resetSections(menuID: MenuID) { + this.store.dispatch(new ResetMenuSectionsAction(menuID)); + } + /** * Check if a given menu is collapsed * @param {MenuID} menuID The ID of the menu that is to be checked @@ -270,7 +278,7 @@ export class MenuService { * @returns {Observable} Emits true when the given section is currently active, false when the given section is currently inactive */ isSectionActive(menuID: MenuID, id: string): Observable { - return this.getMenuSection(menuID, id).pipe(map((section) => section.active)); + return this.getMenuSection(menuID, id).pipe(hasValueOperator(), map((section) => section.active)); } /** From 6d88381ead0933667ebeaef913772d684b88567c Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 16:35:36 +0200 Subject: [PATCH 04/10] 68067: Route data menu sections - tests --- .../admin-sidebar.component.spec.ts | 5 +- src/app/navbar/navbar.component.spec.ts | 6 +- src/app/shared/menu/menu.component.spec.ts | 75 ++++++++++++++++++- src/app/shared/menu/menu.component.ts | 9 +-- src/app/shared/testing/menu-service.stub.ts | 3 + 5 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts index 581d3341da..e3e8d08753 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts @@ -13,6 +13,8 @@ import { AuthService } from '../../core/auth/auth.service'; import { of as observableOf } from 'rxjs'; import { By } from '@angular/platform-browser'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { RouterTestingModule } from '@angular/router/testing'; +import { ActivatedRoute } from '@angular/router'; describe('AdminSidebarComponent', () => { let comp: AdminSidebarComponent; @@ -21,13 +23,14 @@ describe('AdminSidebarComponent', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - imports: [TranslateModule.forRoot(), NoopAnimationsModule], + imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule], declarations: [AdminSidebarComponent], providers: [ { provide: Injector, useValue: {} }, { provide: MenuService, useValue: menuService }, { provide: CSSVariableService, useClass: CSSVariableServiceStub }, { provide: AuthService, useClass: AuthServiceStub }, + { provide: ActivatedRoute, useValue: {} }, { provide: NgbModal, useValue: { open: () => {/*comment*/} diff --git a/src/app/navbar/navbar.component.spec.ts b/src/app/navbar/navbar.component.spec.ts index 206afda003..907ddc8b87 100644 --- a/src/app/navbar/navbar.component.spec.ts +++ b/src/app/navbar/navbar.component.spec.ts @@ -11,6 +11,8 @@ import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { Injector, NO_ERRORS_SCHEMA } from '@angular/core'; import { MenuService } from '../shared/menu/menu.service'; import { MenuServiceStub } from '../shared/testing/menu-service.stub'; +import { ActivatedRoute } from '@angular/router'; +import { RouterTestingModule } from '@angular/router/testing'; let comp: NavbarComponent; let fixture: ComponentFixture; @@ -24,12 +26,14 @@ describe('NavbarComponent', () => { imports: [ TranslateModule.forRoot(), NoopAnimationsModule, - ReactiveFormsModule], + ReactiveFormsModule, + RouterTestingModule], declarations: [NavbarComponent], providers: [ { provide: Injector, useValue: {} }, { provide: MenuService, useValue: menuService }, { provide: HostWindowService, useValue: new HostWindowServiceStub(800) }, + { provide: ActivatedRoute, useValue: {} } ], schemas: [NO_ERRORS_SCHEMA] }) diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index 625c96e795..c277725d20 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/src/app/shared/menu/menu.component.spec.ts @@ -7,19 +7,65 @@ import { MenuComponent } from './menu.component'; import { MenuServiceStub } from '../testing/menu-service.stub'; import { of as observableOf } from 'rxjs'; import { MenuSection } from './menu.reducer'; +import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; +import { RouterTestingModule } from '@angular/router/testing'; +import { MenuItemType } from './initial-menus-state'; +import { LinkMenuItemModel } from './menu-item/models/link.model'; -describe('MenuComponent', () => { +fdescribe('MenuComponent', () => { let comp: MenuComponent; let fixture: ComponentFixture; let menuService: MenuService; + let routeDataMenuSection: MenuSection; + let routeDataMenuChildSection: MenuSection; + let route: any; + let router: any; beforeEach(async(() => { + routeDataMenuSection = { + id: 'mockSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.mockSection', + link: '' + } as LinkMenuItemModel + }; + routeDataMenuChildSection = { + id: 'mockChildSection', + parentID: 'mockSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.mockChildSection', + link: '' + } as LinkMenuItemModel + }; + route = { + root: { + snapshot: { + data: { + menu: routeDataMenuSection + } + }, + firstChild: { + snapshot: { + data: { + menu: routeDataMenuChildSection + } + } + } + } + }; TestBed.configureTestingModule({ - imports: [TranslateModule.forRoot(), NoopAnimationsModule], + imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule], declarations: [MenuComponent], providers: [ { provide: Injector, useValue: {} }, { provide: MenuService, useClass: MenuServiceStub }, + { provide: ActivatedRoute, useValue: route } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(MenuComponent, { @@ -31,11 +77,36 @@ describe('MenuComponent', () => { fixture = TestBed.createComponent(MenuComponent); comp = fixture.componentInstance; // SearchPageComponent test instance menuService = (comp as any).menuService; + router = TestBed.get(Router); spyOn(comp as any, 'getSectionDataInjector').and.returnValue(MenuSection); spyOn(comp as any, 'getSectionComponent').and.returnValue(observableOf({})); fixture.detectChanges(); }); + describe('ngOnInit', () => { + beforeEach(() => { + spyOn(comp, 'resolveMenuSections').and.returnValue([]); + }); + + it('should call resolveMenuSections on init', () => { + router.events = observableOf(new NavigationEnd(0, '', '')); + comp.ngOnInit(); + expect(comp.resolveMenuSections).toHaveBeenCalledWith(route.root); + }) + }); + + describe('resolveMenuSections', () => { + let result: MenuSection[]; + + beforeEach(() => { + result = comp.resolveMenuSections(route.root); + }); + + it('should return the current route\'s menu sections', () => { + expect(result).toEqual([routeDataMenuSection, routeDataMenuChildSection]) + }); + }); + describe('toggle', () => { beforeEach(() => { spyOn(menuService, 'toggleMenu'); diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 4e29ea0ee0..f351329fed 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -1,15 +1,14 @@ import { ChangeDetectionStrategy, Component, Injector, OnInit } from '@angular/core'; import { Observable } from 'rxjs/internal/Observable'; -import { MenuService } from '../../shared/menu/menu.service'; -import { MenuID } from '../../shared/menu/initial-menus-state'; -import { MenuSection } from '../../shared/menu/menu.reducer'; -import { distinctUntilChanged, filter, first, map, switchMap, tap } from 'rxjs/operators'; +import { MenuService } from './menu.service'; +import { MenuID } from './initial-menus-state'; +import { MenuSection } from './menu.reducer'; +import { distinctUntilChanged, filter, first, map, tap } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; import { hasNoValue, hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; -import { of as observableOf } from 'rxjs/internal/observable/of'; /** * A basic implementation of a MenuComponent diff --git a/src/app/shared/testing/menu-service.stub.ts b/src/app/shared/testing/menu-service.stub.ts index de71e3483d..0246aa39d1 100644 --- a/src/app/shared/testing/menu-service.stub.ts +++ b/src/app/shared/testing/menu-service.stub.ts @@ -59,6 +59,9 @@ export class MenuServiceStub { removeSection(): void { /***/ }; + resetSections(): void { /***/ + }; + isMenuVisible(id: MenuID): Observable { return observableOf(true) }; From 8761d0ac275786c97e9940e63f1004ea730fd7d8 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 16:46:58 +0200 Subject: [PATCH 05/10] 68067: Add unsubscribes on open subscriptions in menu component --- src/app/shared/menu/menu.component.spec.ts | 2 +- src/app/shared/menu/menu.component.ts | 28 +++++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index c277725d20..3524dfaae3 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/src/app/shared/menu/menu.component.spec.ts @@ -12,7 +12,7 @@ import { RouterTestingModule } from '@angular/router/testing'; import { MenuItemType } from './initial-menus-state'; import { LinkMenuItemModel } from './menu-item/models/link.model'; -fdescribe('MenuComponent', () => { +describe('MenuComponent', () => { let comp: MenuComponent; let fixture: ComponentFixture; let menuService: MenuService; diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index f351329fed..cf9c2752dd 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectionStrategy, Component, Injector, OnInit } from '@angular/core'; +import { ChangeDetectionStrategy, Component, Injector, OnDestroy, OnInit } from '@angular/core'; import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from './menu.service'; import { MenuID } from './initial-menus-state'; @@ -9,6 +9,7 @@ import { hasNoValue, hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; +import { Subscription } from 'rxjs/internal/Subscription'; /** * A basic implementation of a MenuComponent @@ -17,7 +18,7 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; selector: 'ds-menu', template: '' }) -export class MenuComponent implements OnInit { +export class MenuComponent implements OnInit, OnDestroy { /** * The ID of the Menu (See MenuID) */ @@ -63,6 +64,12 @@ export class MenuComponent implements OnInit { */ private previewTimer; + /** + * Array to track all subscriptions and unsubscribe them onDestroy + * @type {Array} + */ + subs: Subscription[] = []; + constructor(protected menuService: MenuService, protected injector: Injector, protected route: ActivatedRoute, @@ -78,19 +85,19 @@ export class MenuComponent implements OnInit { this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID); this.menuVisible = this.menuService.isMenuVisible(this.menuID); this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(distinctUntilChanged((x, y) => JSON.stringify(x) === JSON.stringify(y))); - this.sections.subscribe((sections: MenuSection[]) => { + this.subs.push(this.sections.subscribe((sections: MenuSection[]) => { sections.forEach((section: MenuSection) => { this.sectionInjectors.set(section.id, this.getSectionDataInjector(section)); this.getSectionComponent(section).pipe(first()).subscribe((constr) => this.sectionComponents.set(section.id, constr)); }); - }); + })); } /** * Initialize all menu sections and add them to the menu's store */ initSections() { - this.router.events.pipe( + this.subs.push(this.router.events.pipe( filter((e): e is NavigationEnd => e instanceof NavigationEnd), tap(() => this.menuService.resetSections(this.menuID)), map(() => this.resolveMenuSections(this.route.root)) @@ -99,7 +106,7 @@ export class MenuComponent implements OnInit { sections.forEach((section: MenuSection) => { this.menuService.addSection(this.menuID, section); }); - }); + })); } /** @@ -212,4 +219,13 @@ export class MenuComponent implements OnInit { parent: this.injector }); } + + /** + * Unsubscribe from open subscriptions + */ + ngOnDestroy(): void { + this.subs + .filter((subscription) => hasValue(subscription)) + .forEach((subscription) => subscription.unsubscribe()); + } } From 79eecc7c767e2197d4000e512a4d804abf3014d8 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 19 May 2020 17:34:56 +0200 Subject: [PATCH 06/10] 68067: Route menu data - menu ID --- src/app/shared/menu/menu.component.spec.ts | 13 ++++++++++--- src/app/shared/menu/menu.component.ts | 6 +++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index 3524dfaae3..901f072dd7 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/src/app/shared/menu/menu.component.spec.ts @@ -9,7 +9,7 @@ import { of as observableOf } from 'rxjs'; import { MenuSection } from './menu.reducer'; import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; -import { MenuItemType } from './initial-menus-state'; +import { MenuID, MenuItemType } from './initial-menus-state'; import { LinkMenuItemModel } from './menu-item/models/link.model'; describe('MenuComponent', () => { @@ -21,6 +21,8 @@ describe('MenuComponent', () => { let route: any; let router: any; + const mockMenuID = 'mock-menuID' as MenuID; + beforeEach(async(() => { routeDataMenuSection = { id: 'mockSection', @@ -47,13 +49,17 @@ describe('MenuComponent', () => { root: { snapshot: { data: { - menu: routeDataMenuSection + menu: { + [mockMenuID]: routeDataMenuSection + } } }, firstChild: { snapshot: { data: { - menu: routeDataMenuChildSection + menu: { + [mockMenuID]: routeDataMenuChildSection + } } } } @@ -76,6 +82,7 @@ describe('MenuComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(MenuComponent); comp = fixture.componentInstance; // SearchPageComponent test instance + comp.menuID = mockMenuID; menuService = (comp as any).menuService; router = TestBed.get(Router); spyOn(comp as any, 'getSectionDataInjector').and.returnValue(MenuSection); diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index cf9c2752dd..074d0f1e95 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -125,11 +125,11 @@ export class MenuComponent implements OnInit, OnDestroy { const data = route.snapshot.data; const last: boolean = hasNoValue(route.firstChild); - if (hasValue(data) && hasValue(data.menu)) { + if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[this.menuID])) { if (!last) { - return [...data.menu, ...this.resolveMenuSections(route.firstChild)] + return [...data.menu[this.menuID], ...this.resolveMenuSections(route.firstChild)] } else { - return [...data.menu]; + return [...data.menu[this.menuID]]; } } From 0b67bd54fbfeddde3c1112226d251bac047c5e56 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 26 May 2020 11:54:33 +0200 Subject: [PATCH 07/10] 68067: Refactoring menu-component resolving route menu data to menu effects --- .../admin-sidebar/admin-sidebar.component.ts | 10 +-- src/app/core/core.effects.ts | 4 +- src/app/navbar/navbar.component.ts | 14 ++-- src/app/shared/menu/menu.actions.ts | 14 ---- src/app/shared/menu/menu.component.ts | 50 +------------ src/app/shared/menu/menu.effects.ts | 72 +++++++++++++++++++ src/app/shared/menu/menu.reducer.ts | 7 +- src/app/shared/menu/menu.service.ts | 20 +++--- 8 files changed, 103 insertions(+), 88 deletions(-) create mode 100644 src/app/shared/menu/menu.effects.ts diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index d3633c86b2..f11313fb6d 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -18,7 +18,6 @@ import { TextMenuItemModel } from '../../shared/menu/menu-item/models/text.model import { MenuComponent } from '../../shared/menu/menu.component'; import { MenuService } from '../../shared/menu/menu.service'; import { CSSVariableService } from '../../shared/sass-helper/sass-helper.service'; -import { ActivatedRoute, Router } from '@angular/router'; /** * Component representing the admin sidebar @@ -60,19 +59,18 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { constructor(protected menuService: MenuService, protected injector: Injector, - protected route: ActivatedRoute, - protected router: Router, private variableService: CSSVariableService, private authService: AuthService, private modalService: NgbModal ) { - super(menuService, injector, route, router); + super(menuService, injector); } /** * Set and calculate all initial values of the instance variables */ ngOnInit(): void { + this.createMenu(); super.ngOnInit(); this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); this.authService.isAuthenticated() @@ -455,7 +453,9 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { index: 10 }, ]; - menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, menuSection)); + menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, { + shouldPersistOnRouteChange: true + }))); } diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index 9f4242cc9a..cd14a527e8 100644 --- a/src/app/core/core.effects.ts +++ b/src/app/core/core.effects.ts @@ -7,6 +7,7 @@ import { ServerSyncBufferEffects } from './cache/server-sync-buffer.effects'; import { ObjectUpdatesEffects } from './data/object-updates/object-updates.effects'; import { RouteEffects } from './services/route.effects'; import { RouterEffects } from './router/router.effects'; +import { MenuEffects } from '../shared/menu/menu.effects'; export const coreEffects = [ RequestEffects, @@ -17,5 +18,6 @@ export const coreEffects = [ ServerSyncBufferEffects, ObjectUpdatesEffects, RouteEffects, - RouterEffects + RouterEffects, + MenuEffects ]; diff --git a/src/app/navbar/navbar.component.ts b/src/app/navbar/navbar.component.ts index 99e44ff489..ae9000352a 100644 --- a/src/app/navbar/navbar.component.ts +++ b/src/app/navbar/navbar.component.ts @@ -7,7 +7,6 @@ import { TextMenuItemModel } from '../shared/menu/menu-item/models/text.model'; import { LinkMenuItemModel } from '../shared/menu/menu-item/models/link.model'; import { HostWindowService } from '../shared/host-window.service'; import { environment } from '../../environments/environment'; -import { ActivatedRoute, Router } from '@angular/router'; /** * Component representing the public navbar @@ -27,11 +26,14 @@ export class NavbarComponent extends MenuComponent { constructor(protected menuService: MenuService, protected injector: Injector, - protected route: ActivatedRoute, - protected router: Router, public windowService: HostWindowService ) { - super(menuService, injector, route, router); + super(menuService, injector); + } + + ngOnInit(): void { + this.createMenu(); + super.ngOnInit(); } /** @@ -91,7 +93,9 @@ export class NavbarComponent extends MenuComponent { } as LinkMenuItemModel }); }); - menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, menuSection)); + menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, { + shouldPersistOnRouteChange: true + }))); } diff --git a/src/app/shared/menu/menu.actions.ts b/src/app/shared/menu/menu.actions.ts index 5b9166641e..00275441d6 100644 --- a/src/app/shared/menu/menu.actions.ts +++ b/src/app/shared/menu/menu.actions.ts @@ -21,7 +21,6 @@ export const MenuActionTypes = { EXPAND_MENU_PREVIEW: type('dspace/menu/EXPAND_MENU_PREVIEW'), ADD_SECTION: type('dspace/menu-section/ADD_SECTION'), REMOVE_SECTION: type('dspace/menu-section/REMOVE_SECTION'), - RESET_SECTIONS: type('dspace/menu-section/RESET_SECTIONS'), SHOW_SECTION: type('dspace/menu-section/SHOW_SECTION'), HIDE_SECTION: type('dspace/menu-section/HIDE_SECTION'), ACTIVATE_SECTION: type('dspace/menu-section/ACTIVATE_SECTION'), @@ -116,18 +115,6 @@ export class ExpandMenuPreviewAction implements Action { } } -/** - * Action used to remove all sections from a certain menu - */ -export class ResetMenuSectionsAction implements Action { - type = MenuActionTypes.RESET_SECTIONS; - menuID: MenuID; - - constructor(menuID: MenuID) { - this.menuID = menuID; - } -} - // MENU SECTION ACTIONS /** * Action used to perform state changes for a section of a certain menu @@ -238,5 +225,4 @@ export type MenuAction = | ToggleActiveMenuSectionAction | CollapseMenuPreviewAction | ExpandMenuPreviewAction - | ResetMenuSectionsAction /* tslint:enable:max-classes-per-file */ diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 074d0f1e95..329fabf703 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -8,7 +8,6 @@ import { GenericConstructor } from '../../core/shared/generic-constructor'; import { hasNoValue, hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; -import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { Subscription } from 'rxjs/internal/Subscription'; /** @@ -70,17 +69,13 @@ export class MenuComponent implements OnInit, OnDestroy { */ subs: Subscription[] = []; - constructor(protected menuService: MenuService, - protected injector: Injector, - protected route: ActivatedRoute, - protected router: Router) { + constructor(protected menuService: MenuService, protected injector: Injector) { } /** * Sets all instance variables to their initial values */ ngOnInit(): void { - this.initSections(); this.menuCollapsed = this.menuService.isMenuCollapsed(this.menuID); this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID); this.menuVisible = this.menuService.isMenuVisible(this.menuID); @@ -93,49 +88,6 @@ export class MenuComponent implements OnInit, OnDestroy { })); } - /** - * Initialize all menu sections and add them to the menu's store - */ - initSections() { - this.subs.push(this.router.events.pipe( - filter((e): e is NavigationEnd => e instanceof NavigationEnd), - tap(() => this.menuService.resetSections(this.menuID)), - map(() => this.resolveMenuSections(this.route.root)) - ).subscribe((sections: MenuSection[]) => { - this.createMenu(); - sections.forEach((section: MenuSection) => { - this.menuService.addSection(this.menuID, section); - }); - })); - } - - /** - * Initialize all static menu sections and items for this menu - * These sections will be available on any route. Route specific sections should be defined in the route's data instead. - */ - createMenu() { - // Override this method to create and add a standard set of menu sections that should be available for the current menu type on all routes - } - - /** - * Resolve menu sections defined in the current route data (including parent routes) - * @param route The route to resolve data for - */ - resolveMenuSections(route: ActivatedRoute): MenuSection[] { - const data = route.snapshot.data; - const last: boolean = hasNoValue(route.firstChild); - - if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[this.menuID])) { - if (!last) { - return [...data.menu[this.menuID], ...this.resolveMenuSections(route.firstChild)] - } else { - return [...data.menu[this.menuID]]; - } - } - - return !last ? this.resolveMenuSections(route.firstChild) : []; - } - /** * Collapse this menu when it's currently expanded, expand it when its currently collapsed * @param {Event} event The user event that triggered this method diff --git a/src/app/shared/menu/menu.effects.ts b/src/app/shared/menu/menu.effects.ts new file mode 100644 index 0000000000..f0a6c3f958 --- /dev/null +++ b/src/app/shared/menu/menu.effects.ts @@ -0,0 +1,72 @@ +import { ActivatedRoute } from '@angular/router'; +import { MenuSection } from './menu.reducer'; +import { hasNoValue, hasValue } from '../empty.util'; +import { Actions, Effect, ofType } from '@ngrx/effects'; +import { MenuService } from './menu.service'; +import { Observable } from 'rxjs/internal/Observable'; +import { Action } from '@ngrx/store'; +import { ROUTER_NAVIGATED } from '@ngrx/router-store'; +import { Injectable } from '@angular/core'; +import { map, take, tap } from 'rxjs/operators'; +import { MenuID } from './initial-menus-state'; + +@Injectable() +export class MenuEffects { + + constructor(private actions$: Actions, + private menuService: MenuService, + private route: ActivatedRoute) { + } + + @Effect({ dispatch: false }) + public buildRouteMenuSections$: Observable = this.actions$ + .pipe( + ofType(ROUTER_NAVIGATED), + tap(() => { + Object.values(MenuID).forEach((menuID) => { + this.buildRouteMenuSections(menuID); + }); + }) + ); + + buildRouteMenuSections(menuID: MenuID) { + this.menuService.getNonPersistentMenuSections(menuID).pipe( + map((sections) => sections.map((section) => section.id)), + take(1) + ).subscribe((shouldNotPersistIDs: string[]) => { + const resolvedSections = this.resolveRouteMenuSections(this.route.root, menuID); + resolvedSections.forEach((section) => { + const index = shouldNotPersistIDs.indexOf(section.id); + if (index > -1) { + shouldNotPersistIDs.splice(index, 1); + } else { + this.menuService.addSection(menuID, section); + } + }); + shouldNotPersistIDs.forEach((id) => { + this.menuService.removeSection(menuID, id); + }); + }); + } + + /** + * Resolve menu sections defined in the current route data (including parent routes) + * @param route The route to resolve data for + * @param menuID The menu to resolve data for + */ + resolveRouteMenuSections(route: ActivatedRoute, menuID: MenuID): MenuSection[] { + const data = route.snapshot.data; + const last: boolean = hasNoValue(route.firstChild); + + if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[menuID])) { + if (!last) { + return [...data.menu[menuID], ...this.resolveRouteMenuSections(route.firstChild, menuID)] + } else { + return [...data.menu[menuID]]; + } + } + + return !last ? this.resolveRouteMenuSections(route.firstChild, menuID) : []; + } + +} diff --git a/src/app/shared/menu/menu.reducer.ts b/src/app/shared/menu/menu.reducer.ts index 85d6847e69..0ef328aa15 100644 --- a/src/app/shared/menu/menu.reducer.ts +++ b/src/app/shared/menu/menu.reducer.ts @@ -6,7 +6,7 @@ import { MenuAction, MenuActionTypes, MenuSectionAction, - RemoveMenuSectionAction, ResetMenuSectionsAction, + RemoveMenuSectionAction, ShowMenuSectionAction, ToggleActiveMenuSectionAction } from './menu.actions'; @@ -58,6 +58,7 @@ export class MenuSection { model: MenuItemModel; index?: number; icon?: string; + shouldPersistOnRouteChange? = false; } /** @@ -118,10 +119,6 @@ export function menusReducer(state: MenusState = initialMenusState, action: Menu case MenuActionTypes.SHOW_SECTION: { return showSection(state, action as ShowMenuSectionAction); } - case MenuActionTypes.RESET_SECTIONS: { - const newMenuState = Object.assign({}, menuState, { sections: {} }); - return Object.assign({}, state, { [action.menuID]: newMenuState }); - } default: { return state; diff --git a/src/app/shared/menu/menu.service.ts b/src/app/shared/menu/menu.service.ts index 9ff4c2f788..fd3ba1ed5c 100644 --- a/src/app/shared/menu/menu.service.ts +++ b/src/app/shared/menu/menu.service.ts @@ -14,7 +14,7 @@ import { ExpandMenuAction, ExpandMenuPreviewAction, HideMenuAction, - RemoveMenuSectionAction, ResetMenuSectionsAction, + RemoveMenuSectionAction, ShowMenuAction, ToggleActiveMenuSectionAction, ToggleMenuAction, @@ -129,6 +129,16 @@ export class MenuService { ); } + /** + * Retrieve menu sections that shouldn't persist on route change + * @param menuID The ID of the menu the sections reside in + */ + getNonPersistentMenuSections(menuID: MenuID): Observable { + return this.getMenu(menuID).pipe( + map((state: MenuState) => Object.values(state.sections).filter((section: MenuSection) => !section.shouldPersistOnRouteChange)) + ); + } + /** * Add a new section to the store * @param {MenuID} menuID The menu to which the new section is to be added @@ -147,14 +157,6 @@ export class MenuService { this.store.dispatch(new RemoveMenuSectionAction(menuID, sectionID)); } - /** - * Remove all sections within a menu from the store - * @param {MenuID} menuID The menu from which the sections are to be removed - */ - resetSections(menuID: MenuID) { - this.store.dispatch(new ResetMenuSectionsAction(menuID)); - } - /** * Check if a given menu is collapsed * @param {MenuID} menuID The ID of the menu that is to be checked From 7b64d6bc3101dcd086ac096b1ea111c87a2cef17 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 26 May 2020 14:08:54 +0200 Subject: [PATCH 08/10] 68067: JSDocs + menu-effects tests --- src/app/shared/menu/menu.component.spec.ts | 76 +----------- src/app/shared/menu/menu.effects.spec.ts | 129 +++++++++++++++++++++ src/app/shared/menu/menu.effects.ts | 12 ++ 3 files changed, 144 insertions(+), 73 deletions(-) create mode 100644 src/app/shared/menu/menu.effects.spec.ts diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index 901f072dd7..2b83ec4414 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/src/app/shared/menu/menu.component.spec.ts @@ -7,71 +7,25 @@ import { MenuComponent } from './menu.component'; import { MenuServiceStub } from '../testing/menu-service.stub'; import { of as observableOf } from 'rxjs'; import { MenuSection } from './menu.reducer'; -import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; +import { Router } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; -import { MenuID, MenuItemType } from './initial-menus-state'; -import { LinkMenuItemModel } from './menu-item/models/link.model'; +import { MenuID } from './initial-menus-state'; describe('MenuComponent', () => { let comp: MenuComponent; let fixture: ComponentFixture; let menuService: MenuService; - let routeDataMenuSection: MenuSection; - let routeDataMenuChildSection: MenuSection; - let route: any; let router: any; const mockMenuID = 'mock-menuID' as MenuID; beforeEach(async(() => { - routeDataMenuSection = { - id: 'mockSection', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.mockSection', - link: '' - } as LinkMenuItemModel - }; - routeDataMenuChildSection = { - id: 'mockChildSection', - parentID: 'mockSection', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.mockChildSection', - link: '' - } as LinkMenuItemModel - }; - route = { - root: { - snapshot: { - data: { - menu: { - [mockMenuID]: routeDataMenuSection - } - } - }, - firstChild: { - snapshot: { - data: { - menu: { - [mockMenuID]: routeDataMenuChildSection - } - } - } - } - } - }; TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule], declarations: [MenuComponent], providers: [ { provide: Injector, useValue: {} }, - { provide: MenuService, useClass: MenuServiceStub }, - { provide: ActivatedRoute, useValue: route } + { provide: MenuService, useClass: MenuServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(MenuComponent, { @@ -90,30 +44,6 @@ describe('MenuComponent', () => { fixture.detectChanges(); }); - describe('ngOnInit', () => { - beforeEach(() => { - spyOn(comp, 'resolveMenuSections').and.returnValue([]); - }); - - it('should call resolveMenuSections on init', () => { - router.events = observableOf(new NavigationEnd(0, '', '')); - comp.ngOnInit(); - expect(comp.resolveMenuSections).toHaveBeenCalledWith(route.root); - }) - }); - - describe('resolveMenuSections', () => { - let result: MenuSection[]; - - beforeEach(() => { - result = comp.resolveMenuSections(route.root); - }); - - it('should return the current route\'s menu sections', () => { - expect(result).toEqual([routeDataMenuSection, routeDataMenuChildSection]) - }); - }); - describe('toggle', () => { beforeEach(() => { spyOn(menuService, 'toggleMenu'); diff --git a/src/app/shared/menu/menu.effects.spec.ts b/src/app/shared/menu/menu.effects.spec.ts new file mode 100644 index 0000000000..11b468eded --- /dev/null +++ b/src/app/shared/menu/menu.effects.spec.ts @@ -0,0 +1,129 @@ +import { MenuID, MenuItemType } from './initial-menus-state'; +import { MenuSection } from './menu.reducer'; +import { LinkMenuItemModel } from './menu-item/models/link.model'; +import { TestBed } from '@angular/core/testing'; +import { MenuService } from './menu.service'; +import { ActivatedRoute } from '@angular/router'; +import { Observable } from 'rxjs/internal/Observable'; +import { provideMockActions } from '@ngrx/effects/testing'; +import { of as observableOf } from 'rxjs'; +import { cold, hot } from 'jasmine-marbles'; +import { ROUTER_NAVIGATED } from '@ngrx/router-store'; +import { MenuEffects } from './menu.effects'; + +describe('MenuEffects', () => { + let menuEffects: MenuEffects; + let routeDataMenuSection: MenuSection; + let routeDataMenuChildSection: MenuSection; + let toBeRemovedMenuSection: MenuSection; + let alreadyPresentMenuSection: MenuSection; + let route; + let menuService; + let actions: Observable; + + function init() { + routeDataMenuSection = { + id: 'mockSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.mockSection', + link: '' + } as LinkMenuItemModel + }; + routeDataMenuChildSection = { + id: 'mockChildSection', + parentID: 'mockSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.mockChildSection', + link: '' + } as LinkMenuItemModel + }; + toBeRemovedMenuSection = { + id: 'toBeRemovedSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.toBeRemovedSection', + link: '' + } as LinkMenuItemModel + }; + alreadyPresentMenuSection = { + id: 'alreadyPresentSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.alreadyPresentSection', + link: '' + } as LinkMenuItemModel + }; + route = { + root: { + snapshot: { + data: { + menu: { + [MenuID.PUBLIC]: [routeDataMenuSection, alreadyPresentMenuSection] + } + } + }, + firstChild: { + snapshot: { + data: { + menu: { + [MenuID.PUBLIC]: routeDataMenuChildSection + } + } + } + } + } + }; + + menuService = jasmine.createSpyObj('menuService', { + getNonPersistentMenuSections: observableOf([toBeRemovedMenuSection, alreadyPresentMenuSection]), + addSection: {}, + removeSection: {} + }); + } + + beforeEach(() => { + init(); + TestBed.configureTestingModule({ + providers: [ + MenuEffects, + { provide: MenuService, useValue: menuService }, + { provide: ActivatedRoute, useValue: route }, + provideMockActions(() => actions) + ] + }); + + menuEffects = TestBed.get(MenuEffects); + }); + + describe('buildRouteMenuSections$', () => { + it('should add and remove menu sections depending on the current route', () => { + actions = hot('--a-', { + a: { + type: ROUTER_NAVIGATED + } + }); + + const expected = cold('--b-', { + b: { + type: ROUTER_NAVIGATED + } + }); + + expect(menuEffects.buildRouteMenuSections$).toBeObservable(expected); + expect(menuService.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuSection); + expect(menuService.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuChildSection); + expect(menuService.addSection).not.toHaveBeenCalledWith(MenuID.PUBLIC, alreadyPresentMenuSection); + expect(menuService.removeSection).toHaveBeenCalledWith(MenuID.PUBLIC, toBeRemovedMenuSection.id); + }); + }); +}); diff --git a/src/app/shared/menu/menu.effects.ts b/src/app/shared/menu/menu.effects.ts index f0a6c3f958..92df41caff 100644 --- a/src/app/shared/menu/menu.effects.ts +++ b/src/app/shared/menu/menu.effects.ts @@ -10,6 +10,9 @@ import { Injectable } from '@angular/core'; import { map, take, tap } from 'rxjs/operators'; import { MenuID } from './initial-menus-state'; +/** + * Effects modifying the state of menus + */ @Injectable() export class MenuEffects { @@ -18,6 +21,9 @@ export class MenuEffects { private route: ActivatedRoute) { } + /** + * On route change, build menu sections for every menu type depending on the current route data + */ @Effect({ dispatch: false }) public buildRouteMenuSections$: Observable = this.actions$ .pipe( @@ -29,6 +35,12 @@ export class MenuEffects { }) ); + /** + * Build menu sections depending on the current route + * - Adds sections found in the current route data that aren't active yet + * - Removes sections that are active, but not present in the current route data + * @param menuID The menu to add/remove sections to/from + */ buildRouteMenuSections(menuID: MenuID) { this.menuService.getNonPersistentMenuSections(menuID).pipe( map((sections) => sections.map((section) => section.id)), From 9a4fc65331cb0d841c15eb011f0637bf2f6a6216 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 11 Jun 2020 10:50:39 +0200 Subject: [PATCH 09/10] 68067: Fix instance field declaration --- src/app/shared/menu/menu.effects.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/shared/menu/menu.effects.ts b/src/app/shared/menu/menu.effects.ts index 92df41caff..be314cfd49 100644 --- a/src/app/shared/menu/menu.effects.ts +++ b/src/app/shared/menu/menu.effects.ts @@ -16,11 +16,6 @@ import { MenuID } from './initial-menus-state'; @Injectable() export class MenuEffects { - constructor(private actions$: Actions, - private menuService: MenuService, - private route: ActivatedRoute) { - } - /** * On route change, build menu sections for every menu type depending on the current route data */ @@ -35,6 +30,11 @@ export class MenuEffects { }) ); + constructor(private actions$: Actions, + private menuService: MenuService, + private route: ActivatedRoute) { + } + /** * Build menu sections depending on the current route * - Adds sections found in the current route data that aren't active yet From 4fa8d02fb242d9d0667d66b3a2eb3875b6bea96c Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 2 Jul 2020 17:32:34 +0200 Subject: [PATCH 10/10] remove unused imports --- src/app/shared/menu/menu.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 329fabf703..28e906c456 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -3,9 +3,9 @@ import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from './menu.service'; import { MenuID } from './initial-menus-state'; import { MenuSection } from './menu.reducer'; -import { distinctUntilChanged, filter, first, map, tap } from 'rxjs/operators'; +import { distinctUntilChanged, first, map } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { hasNoValue, hasValue } from '../empty.util'; +import { hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; import { Subscription } from 'rxjs/internal/Subscription';