97732 Fixed bug where ContextHelpWrapperComponent subscriptions would not properly be initialized if the application launches with help icons hidden by default

2025-10-07 01:54:15 +00:00 · 2023-01-05 12:06:14 +01:00
parent e16a3edf78
commit 920e36ecf2
12 changed files with 56 additions and 41 deletions
--- 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'
                               }"
--- 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>
--- 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);
+  }
 }
--- 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();
  }
 }
--- 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/>
--- 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);
 }
--- 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] = [];
+    }
+  }
 }
-
-
-
--- 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);
--- 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) {
--- 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",
--- 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">
--- 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>
+</nav>