From 920e36ecf28620571ac28731d4a9ae97397f2b3d Mon Sep 17 00:00:00 2001
From: Koen Pauwels <koen.pauwels@atmire.com>
Date: Thu, 5 Jan 2023 12:06:14 +0100
Subject: [PATCH] 97732 Fixed bug where ContextHelpWrapperComponent
 subscriptions would not properly be initialized if the application launches
 with help icons hidden by default

---
 src/app/footer/footer.component.html          |  1 +
 .../context-help-toggle.component.html        |  4 +-
 .../context-help-toggle.component.scss        |  6 +-
 .../context-help-toggle.component.ts          |  5 +-
 .../page-internal-server-error.component.html |  1 -
 .../context-help-wrapper.component.scss       |  1 -
 .../context-help-wrapper.component.ts         | 68 +++++++++++--------
 src/app/shared/context-help.directive.ts      |  2 +-
 src/app/shared/context-help.reducer.ts        |  3 +-
 src/assets/i18n/en.json5                      |  2 +
 .../dspace/app/header/header.component.html   |  1 +
 .../dspace/app/navbar/navbar.component.html   |  3 +-
 12 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/src/app/footer/footer.component.html b/src/app/footer/footer.component.html
index 58f264a5ef..8f991b9e41 100644
--- a/src/app/footer/footer.component.html
+++ b/src/app/footer/footer.component.html
@@ -65,6 +65,7 @@
       <ul class="footer-info list-unstyled small d-flex justify-content-center mb-0">
         <li>
             <a *dsContextHelp="{content: 'context-help.multi-para.test',
+                                id: 'show-cookie-settings',
                                 tooltipPlacement: 'top',
                                 iconPlacement: 'left'
                                }"
diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.html b/src/app/header/context-help-toggle/context-help-toggle.component.html
index caf1bf5daf..9fba8fa032 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.html
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.html
@@ -2,7 +2,9 @@
   <a href="javascript:void(0);"
      role="button"
      (click)="onClick()"
+     [attr.aria-label]="'nav.context-help-toggle' | translate"
+     [title]="'nav.context-help-toggle' | translate"
   >
-    <i class="fas fa-question-circle ds-context-help-toggle"></i>
+    <i class="fas fa-lg fa-fw fa-question-circle ds-context-help-toggle"></i>
   </a>
 </div>
diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.scss b/src/app/header/context-help-toggle/context-help-toggle.component.scss
index ba34ba8b89..89f0c65bd8 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.scss
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.scss
@@ -1,8 +1,8 @@
 .ds-context-help-toggle {
   color: var(--ds-header-icon-color);
   background-color: var(--ds-header-bg);
-}
 
-.ds-context-help-toggle:hover {
-  color: var(--ds-header-icon-color-hover);
+  &:hover, &focus {
+    color: var(--ds-header-icon-color-hover);
+  }
 }
diff --git a/src/app/header/context-help-toggle/context-help-toggle.component.ts b/src/app/header/context-help-toggle/context-help-toggle.component.ts
index cc5e779d6e..2956d3ef4c 100644
--- a/src/app/header/context-help-toggle/context-help-toggle.component.ts
+++ b/src/app/header/context-help-toggle/context-help-toggle.component.ts
@@ -1,5 +1,6 @@
 import { Component, OnInit } from '@angular/core';
 import { ContextHelpService } from '../../shared/context-help.service';
+import { TranslateService } from '@ngx-translate/core';
 
 @Component({
   selector: 'ds-context-help-toggle',
@@ -9,14 +10,14 @@ import { ContextHelpService } from '../../shared/context-help.service';
 export class ContextHelpToggleComponent implements OnInit {
 
   constructor(
-    private contextHelpService: ContextHelpService
+    private contextHelpService: ContextHelpService,
+    private translateService: TranslateService
   ) { }
 
   ngOnInit(): void {
   }
 
   onClick() {
-    console.log('toggling icons');
     this.contextHelpService.toggleIcons();
   }
 }
diff --git a/src/app/page-internal-server-error/page-internal-server-error.component.html b/src/app/page-internal-server-error/page-internal-server-error.component.html
index fe72c78d92..bb43cdc678 100644
--- a/src/app/page-internal-server-error/page-internal-server-error.component.html
+++ b/src/app/page-internal-server-error/page-internal-server-error.component.html
@@ -1,5 +1,4 @@
 <div class="page-internal-server-error container">
-  <ds-context-help-toggle></ds-context-help-toggle>
   <h1>500</h1>
   <h2><small *dsContextHelp="{content: 'context-help.multi-para.test', id: 'server-error'}">{{"500.page-internal-server-error" | translate}}</small></h2>
   <br/>
diff --git a/src/app/shared/context-help-wrapper/context-help-wrapper.component.scss b/src/app/shared/context-help-wrapper/context-help-wrapper.component.scss
index d945548ca4..3bf9c7bba8 100644
--- a/src/app/shared/context-help-wrapper/context-help-wrapper.component.scss
+++ b/src/app/shared/context-help-wrapper/context-help-wrapper.component.scss
@@ -13,7 +13,6 @@
   border-radius: 50%;
 }
 
-
 .ds-context-help-icon-left {
     left: var(--ds-context-x-offset);
 }
diff --git a/src/app/shared/context-help-wrapper/context-help-wrapper.component.ts b/src/app/shared/context-help-wrapper/context-help-wrapper.component.ts
index 4639c12086..68f2ac0002 100644
--- a/src/app/shared/context-help-wrapper/context-help-wrapper.component.ts
+++ b/src/app/shared/context-help-wrapper/context-help-wrapper.component.ts
@@ -19,7 +19,7 @@ import { ContextHelp } from '../context-help.model';
   templateUrl: './context-help-wrapper.component.html',
   styleUrls: ['./context-help-wrapper.component.scss'],
 })
-export class ContextHelpWrapperComponent implements OnInit, AfterViewInit, OnDestroy {
+export class ContextHelpWrapperComponent implements OnInit, OnDestroy {
   /**
    * Template reference for the wrapped element.
    */
@@ -46,11 +46,9 @@ export class ContextHelpWrapperComponent implements OnInit, AfterViewInit, OnDes
    */
   @Input() dontParseLinks?: boolean;
 
-  @ViewChild('tooltip', { static: false }) tooltip: NgbTooltip;
-
   shouldShowIcon$: Observable<boolean>;
 
-  private subs: Subscription[] = [];
+  tooltip: NgbTooltip;
 
   // TODO: dependent on evaluation order of input setters?
   parsedContent$: Observable<(string | {href: string, text: string})[]> = observableOf([]);
@@ -62,6 +60,9 @@ export class ContextHelpWrapperComponent implements OnInit, AfterViewInit, OnDes
     );
   }
 
+  private subs: {always: Subscription[], tooltipBound: Subscription[]}
+    = {always: [], tooltipBound: []};
+
   constructor(
     private translateService: TranslateService,
     private contextHelpService: ContextHelpService
@@ -69,35 +70,37 @@ export class ContextHelpWrapperComponent implements OnInit, AfterViewInit, OnDes
 
   ngOnInit() {
     this.shouldShowIcon$ = this.contextHelpService.shouldShowIcons$();
-    this.subs.push(this.shouldShowIcon$.subscribe());
+    this.subs.always = [this.shouldShowIcon$.subscribe()];
   }
 
-  ngAfterViewInit() {
-    this.subs.push(
-      this.contextHelpService.getContextHelp$(this.id)
-        .pipe(hasValueOperator())
-        .subscribe((ch: ContextHelp) => {
-          if (ch.isTooltipVisible && !this.tooltip.isOpen()) {
-            this.tooltip.open();
-          } else if (!ch.isTooltipVisible && this.tooltip.isOpen()) {
-            this.tooltip.close()
-          }
+  @ViewChild('tooltip', { static: false }) set setTooltip(tooltip: NgbTooltip) {
+    this.tooltip = tooltip;
+    this.clearSubs('tooltipBound');
+    if (this.tooltip !== undefined) {
+      this.subs.tooltipBound = [
+        this.contextHelpService.getContextHelp$(this.id)
+          .pipe(hasValueOperator())
+          .subscribe((ch: ContextHelp) => {
+            if (ch.isTooltipVisible && !this.tooltip.isOpen()) {
+              this.tooltip.open();
+            } else if (!ch.isTooltipVisible && this.tooltip.isOpen()) {
+              this.tooltip.close();
+            }
+          }),
+
+        this.tooltip.shown.subscribe(() => {
+          this.contextHelpService.showTooltip(this.id);
         }),
 
-      this.tooltip.shown.subscribe(() => {
-        this.contextHelpService.showTooltip(this.id);
-      }),
-
-      this.tooltip.hidden.subscribe(() => {
-        this.contextHelpService.hideTooltip(this.id);
-      })
-    );
+        this.tooltip.hidden.subscribe(() => {
+          this.contextHelpService.hideTooltip(this.id);
+        })
+      ];
+    }
   }
 
   ngOnDestroy() {
-    for (let sub of this.subs) {
-      sub.unsubscribe();
-    }
+    this.clearSubs();
   }
 
   onClick() {
@@ -146,7 +149,14 @@ export class ContextHelpWrapperComponent implements OnInit, AfterViewInit, OnDes
         : ({href: match[2], text: match[1]});
     });
   }
+
+  private clearSubs(filter: null | 'tooltipBound' = null) {
+    if (filter === null) {
+      [].concat(...Object.values(this.subs)).forEach(sub => sub.unsubscribe());
+      this.subs = {always: [], tooltipBound: []};
+    } else {
+      this.subs[filter].forEach(sub => sub.unsubscribe());
+      this.subs[filter] = [];
+    }
+  }
 }
-
-
-
diff --git a/src/app/shared/context-help.directive.ts b/src/app/shared/context-help.directive.ts
index 89e283715a..56e7514b98 100644
--- a/src/app/shared/context-help.directive.ts
+++ b/src/app/shared/context-help.directive.ts
@@ -52,7 +52,7 @@ export class ContextHelpDirective implements OnChanges, OnDestroy {
   ngOnChanges() {
     this.clearMostRecentId();
     this.mostRecentId = this.dsContextHelp.id;
-    this.contextHelpService.add({id: this.dsContextHelp.id}); // TODO: tooltipVisible field
+    this.contextHelpService.add({id: this.dsContextHelp.id, isTooltipVisible: false});
 
     const factory
       = this.componentFactoryResolver.resolveComponentFactory(ContextHelpWrapperComponent);
diff --git a/src/app/shared/context-help.reducer.ts b/src/app/shared/context-help.reducer.ts
index 5176da0adc..13e9488dfd 100644
--- a/src/app/shared/context-help.reducer.ts
+++ b/src/app/shared/context-help.reducer.ts
@@ -10,8 +10,7 @@ export interface ContextHelpState {
   models: ContextHelpModels;
 }
 
-// TODO
-const initialState: ContextHelpState = {allIconsVisible: true, models: {}};
+const initialState: ContextHelpState = {allIconsVisible: false, models: {}};
 
 export function contextHelpReducer(state: ContextHelpState = initialState, action: ContextHelpAction): ContextHelpState {
   switch (action.type) {
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index 4151e4202d..473dab6090 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -2674,6 +2674,8 @@
 
   "nav.community-browse.header": "By Community",
 
+  "nav.context-help-toggle": "Toggle context help",
+
   "nav.language": "Language switch",
 
   "nav.login": "Log In",
diff --git a/src/themes/dspace/app/header/header.component.html b/src/themes/dspace/app/header/header.component.html
index cf691ea6c4..43c252035d 100644
--- a/src/themes/dspace/app/header/header.component.html
+++ b/src/themes/dspace/app/header/header.component.html
@@ -8,6 +8,7 @@
     <div class="d-flex flex-grow-1 ml-auto justify-content-end align-items-center">
       <ds-search-navbar class="navbar-search"></ds-search-navbar>
       <ds-lang-switch></ds-lang-switch>
+      <ds-context-help-toggle></ds-context-help-toggle>
       <ds-auth-nav-menu></ds-auth-nav-menu>
       <ds-impersonate-navbar></ds-impersonate-navbar>
       <div class="pl-2">
diff --git a/src/themes/dspace/app/navbar/navbar.component.html b/src/themes/dspace/app/navbar/navbar.component.html
index f061c7cb3b..75f693fcfa 100644
--- a/src/themes/dspace/app/navbar/navbar.component.html
+++ b/src/themes/dspace/app/navbar/navbar.component.html
@@ -14,7 +14,8 @@
     </div>
     <ds-search-navbar class="navbar-collapsed"></ds-search-navbar>
     <ds-lang-switch class="navbar-collapsed"></ds-lang-switch>
+    <ds-context-help-toggle class="navbar-collapsed"></ds-context-help-toggle>
     <ds-auth-nav-menu class="navbar-collapsed"></ds-auth-nav-menu>
     <ds-impersonate-navbar class="navbar-collapsed"></ds-impersonate-navbar>
   </div>
-</nav>
\ No newline at end of file
+</nav>