From 0d18c66416ad4de9c60e2d26d72643c27f46770a Mon Sep 17 00:00:00 2001 From: Milan Klanjsek Date: Thu, 15 Oct 2020 11:09:32 -0600 Subject: [PATCH] Navigation: addressed feedback, simplifued update logic Signed-off-by: Milan Klanjsek --- internal/module/module.go | 2 +- .../services/navigation/navigation.service.ts | 25 +++-- .../navigation/navigation.test.data.ts | 10 +- .../navigation/navigation.component.html | 12 +-- .../navigation/navigation.component.scss | 6 +- .../smart/navigation/navigation.component.ts | 98 ++++++------------- 6 files changed, 62 insertions(+), 91 deletions(-) diff --git a/internal/module/module.go b/internal/module/module.go index 4f790d4a31..7b5896ada5 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -28,7 +28,7 @@ type ContentOptions struct { type Module interface { // Name is the name of the module. Name() string - // Description + // Description is a description for this module Description() string // ClientRequestHandlers are handlers for handling client requests. ClientRequestHandlers() []octant.ClientRequestHandler diff --git a/web/src/app/modules/shared/services/navigation/navigation.service.ts b/web/src/app/modules/shared/services/navigation/navigation.service.ts index 25b4860dc3..1649fecfd0 100644 --- a/web/src/app/modules/shared/services/navigation/navigation.service.ts +++ b/web/src/app/modules/shared/services/navigation/navigation.service.ts @@ -40,7 +40,6 @@ export class NavigationService { current = new BehaviorSubject(emptyNavigation); modules = new BehaviorSubject([]); selectedItem = new BehaviorSubject({ module: 0, index: -1 }); - public expandedState: BehaviorSubject = new BehaviorSubject({}); public collapsed: BehaviorSubject = new BehaviorSubject( false ); @@ -84,17 +83,19 @@ export class NavigationService { ); } - if ( - (suggested.index >= 0 && - suggested.module !== this.selectedItem.value.module) || - suggested.index !== this.selectedItem.value.index - ) { - this.selectedItem.next(suggested); + if (suggested.index < 0) { + suggested.index = 0; } + + this.selectedItem.next(suggested); } indexFromUrl(url: string): Selection { const strippedUrl = this.stripUrl(url); + if (strippedUrl.length === 0) { + return { module: 1, index: 0 }; + } + for (const [moduleIndex, module] of this.modules.value.entries()) { const modulePath = this.stripUrl(module.path); @@ -108,8 +109,6 @@ export class NavigationService { if (child.children) { for (const grandchild of child.children) { if (strippedUrl === grandchild.path) { - this.expandedState[childIndex] = true; - this.expandedState.next(this.expandedState); return { module: moduleIndex, index: childIndex }; } } @@ -126,6 +125,7 @@ export class NavigationService { createModules(sections: any[]) { const modules: Module[] = []; + let pluginsIndex = 3; sections.forEach((section, index) => { if (section.module && section.module.length > 0) { @@ -139,12 +139,16 @@ export class NavigationService { }); } }); + modules.forEach((module, index) => { module.children = []; module.endIndex = index === modules.length - 1 ? sections.length - 1 : modules[index + 1].startIndex; + if (module.name === 'configuration') { + pluginsIndex = index; + } if (sections[module.startIndex].children) { if (module.path !== sections[module.startIndex].children[0].path) { const first = { @@ -166,6 +170,9 @@ export class NavigationService { } } }); + if (modules.length > 0) { + modules.push(modules.splice(pluginsIndex, 1)[0]); + } this.modules.next(modules); } diff --git a/web/src/app/modules/shared/services/navigation/navigation.test.data.ts b/web/src/app/modules/shared/services/navigation/navigation.test.data.ts index ee6e80a304..fb0fd4b057 100644 --- a/web/src/app/modules/shared/services/navigation/navigation.test.data.ts +++ b/web/src/app/modules/shared/services/navigation/navigation.test.data.ts @@ -379,9 +379,9 @@ export const expectedSelection = { 'cluster-overview/storage': { module: 2, index: 5 }, 'cluster-overview/storage/persistent-volumes': { module: 2, index: 5 }, 'cluster-overview/port-forward': { module: 2, index: 6 }, - 'configuration/plugins': { module: 3, index: 0 }, - 'argo-ui': { module: 4, index: 0 }, - 'plugin-name': { module: 5, index: 0 }, - 'plugin-name/nested-once': { module: 5, index: 1 }, - openstack: { module: 6, index: 0 }, + 'configuration/plugins': { module: 6, index: 0 }, + 'argo-ui': { module: 3, index: 0 }, + 'plugin-name': { module: 4, index: 0 }, + 'plugin-name/nested-once': { module: 4, index: 1 }, + openstack: { module: 5, index: 0 }, }; diff --git a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.html b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.html index a4947e9bdf..45cc67690a 100644 --- a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.html +++ b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.html @@ -46,7 +46,7 @@ }">
- {{ getModuleTitle() }} + {{ currentModule?.title }}
- {{ getModuleDescription() }} + {{ currentModule?.description }}
diff --git a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.scss b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.scss index 63ed6e6461..4ac9b45331 100644 --- a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.scss +++ b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.scss @@ -59,6 +59,10 @@ .module-tabs-labels { width: 76px; margin-right: 0px; + + ::ng-deep ul { + padding-top: 0.4rem; + } } .button-tool.active, @@ -189,7 +193,7 @@ flex-direction: column; flex-grow: 1; overflow-y: hidden; - padding-top: 0.6rem; + padding-top: 0.4rem; border-left: 1px solid rgba(173, 187, 196, 0.3); } diff --git a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.ts b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.ts index 73a4b454c6..4cf411897b 100644 --- a/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.ts +++ b/web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.ts @@ -34,13 +34,16 @@ const emptyNavigation: Navigation = { export class NavigationComponent implements OnInit, OnDestroy { collapsed = false; showLabels = true; - navExpandedState: any; selectedItem: Selection = { module: 0, index: -1 }; flyoutIndex = -1; navigation = emptyNavigation; modules: Module[] = []; + currentModule: Module; - private navigationSubscription: Subscription; + private subscriptionModules: Subscription; + private subscriptionSelectedItem: Subscription; + private subscriptionCollapsed: Subscription; + private subscriptionShowLabels: Subscription; constructor( private iconService: IconService, @@ -51,35 +54,28 @@ export class NavigationComponent implements OnInit, OnDestroy { ) {} ngOnInit() { - this.navigationSubscription = this.navigationService.modules.subscribe( + this.subscriptionModules = this.navigationService.modules.subscribe( modules => { this.modules = modules; + this.currentModule = this.modules[this.selectedItem.module]; this.cd.markForCheck(); } ); - this.navigationSubscription = this.navigationService.selectedItem.subscribe( + this.subscriptionSelectedItem = this.navigationService.selectedItem.subscribe( selection => { if ( this.selectedItem.index !== selection.index || this.selectedItem.module !== selection.module ) { this.selectedItem = selection; + this.currentModule = this.modules[this.selectedItem.module]; this.cd.markForCheck(); } } ); - this.navigationSubscription = this.navigationService.expandedState.subscribe( - state => { - if (this.navExpandedState !== state) { - this.navExpandedState = state; - this.cd.markForCheck(); - } - } - ); - - this.navigationSubscription = this.navigationService.collapsed.subscribe( + this.subscriptionCollapsed = this.navigationService.collapsed.subscribe( col => { if (this.collapsed !== col) { this.collapsed = col; @@ -88,7 +84,7 @@ export class NavigationComponent implements OnInit, OnDestroy { } ); - this.navigationSubscription = this.navigationService.showLabels.subscribe( + this.subscriptionShowLabels = this.navigationService.showLabels.subscribe( col => { if (this.showLabels !== col) { this.showLabels = col; @@ -101,10 +97,16 @@ export class NavigationComponent implements OnInit, OnDestroy { @HostListener('window:keyup', ['$event']) keyEvent(event: KeyboardEvent) { if (event.key === 'T' && event.ctrlKey) { + event.preventDefault(); + event.cancelBubble = true; this.themeService.switchTheme(); } else if (event.key === 'N' && event.ctrlKey) { + event.preventDefault(); + event.cancelBubble = true; this.updateNavCollapsed(!this.collapsed); } else if (event.key === 'L' && event.ctrlKey) { + event.preventDefault(); + event.cancelBubble = true; this.navigationService.showLabels.next(!this.showLabels); } } @@ -115,23 +117,11 @@ export class NavigationComponent implements OnInit, OnDestroy { : index.toString(); } - getSelectedSections() { - const currentModule = this.modules[this.selectedItem.module]; - return currentModule ? currentModule.children : []; - } - - getModuleTitle() { - const currentModule = this.modules[this.selectedItem.module]; - return currentModule ? currentModule.title + ' Module' : ''; - } - - getModuleDescription() { - const currentModule = this.modules[this.selectedItem.module]; - return currentModule ? currentModule.description : ''; - } - ngOnDestroy(): void { - this.navigationSubscription.unsubscribe(); + this.subscriptionModules.unsubscribe(); + this.subscriptionSelectedItem.unsubscribe(); + this.subscriptionCollapsed.unsubscribe(); + this.subscriptionShowLabels.unsubscribe(); } identifyNavigationItem(index: number, item: NavigationChild): string { @@ -151,71 +141,41 @@ export class NavigationComponent implements OnInit, OnDestroy { } openPopup(index: number) { - this.clearExpandedState(); this.setNavState(true, index); this.setLastSelection(index); } closePopups(index) { - this.clearExpandedState(); this.flyoutIndex = -1; this.setLastSelection(index); } - setExpandedState(index, state) { - this.navExpandedState[index] = state; - this.navigationService.expandedState.next(this.navExpandedState); - } - - clearExpandedState() { - this.navExpandedState = {}; - this.navigationService.expandedState.next(this.navExpandedState); - } - setNavState($event, state: number) { - if (this.collapsed) { + if ($event) { this.setLastSelection(state); - } else { - this.setExpandedState(state, $event); - if ($event && this.selectedItem.index !== state) { - // collapse previously selected group - if (this.selectedItem) { - this.setExpandedState(this.selectedItem.index, false); - } - this.setLastSelection(state); - } } } shouldExpand(index: number) { if (this.collapsed) { return index === this.flyoutIndex; - } else if (index.toString() in this.navExpandedState) { - return this.navExpandedState[index]; - } - return false; + } else return index === this.selectedItem.index; } updateNavCollapsed(value: boolean): void { - this.collapsed = value; this.navigationService.collapsed.next(value); - this.setExpandedState(this.selectedItem.index, false); } setLastSelection(index) { - this.selectedItem.index = index; - this.navigationService.selectedItem.next({ - module: this.selectedItem.module, - index, - }); + if (this.selectedItem.index !== index) { + this.navigationService.selectedItem.next({ + module: this.selectedItem.module, + index, + }); + } } setModule(module: number): void { - if (this.selectedItem) { - this.setExpandedState(this.selectedItem.index, false); - } - - this.selectedItem.module = module; this.navigationService.selectedItem.next({ module, index: this.selectedItem.index,