Navigation: addressed feedback, simplifued update logic

Signed-off-by: Milan Klanjsek <mklanjsek@pivotal.io>

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

web/src/app/modules/shared/services/navigation/navigation.service.ts

@@ -40,7 +40,6 @@ export class NavigationService {
  current = new BehaviorSubject<Navigation>(emptyNavigation);
  modules = new BehaviorSubject<Module[]>([]);
  selectedItem = new BehaviorSubject<Selection>({ module: 0, index: -1 });
- public expandedState: BehaviorSubject<any> = new BehaviorSubject<any>({});
  public collapsed: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(
  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);
  }
 

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 },
 };

web/src/app/modules/sugarloaf/components/smart/navigation/navigation.component.html

@@ -46,7 +46,7 @@
  }">
  <ng-container
  *ngFor="
- let section of getSelectedSections();
+ let section of currentModule?.children;
  let i = index;
  trackBy: identifyNavigationItem
  "
@@ -92,7 +92,7 @@
  >
  <a
  [routerLink]="formatPath(category.path)"
- [routerLinkActiveOptions]="{ exact: true }"
+ [routerLinkActiveOptions]="{ exact: false }"
  routerLinkActive="active"
  class="flyout-item nav-link"
  (click)="setNavState(false, i)"
@@ -136,7 +136,7 @@
  <a
  clrVerticalNavLink
  [routerLink]="formatPath(category.path)"
- [routerLinkActiveOptions]="{ exact: true }"
+ [routerLinkActiveOptions]="{ exact: false }"
  routerLinkActive="active"
  >
  <clr-icon
@@ -155,7 +155,7 @@
  clrVerticalNavLink
  (click)="closePopups(i)"
  [routerLink]="formatPath(section.path)"
- [routerLinkActiveOptions]="{ exact: true }"
+ [routerLinkActiveOptions]="{ exact: i==0 }"
  routerLinkActive="active"
  >
  <clr-icon *ngIf="section.iconName"
@@ -177,11 +177,11 @@
  <div class="clr-col-12">
  <div class="card">
  <div class="card-header">
- {{ getModuleTitle() }}
+ {{ currentModule?.title }}
  </div>
  <div class="card-block">
  <div class="card-text">
- {{ getModuleDescription() }}
+ {{ currentModule?.description }}
  </div>
  </div>
  </div>

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);
  }
 

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,