Merge pull request #2395 from alexandrevryghem/themed-component-fixes…

…-main Fix themed components duplicating themself when switching themes
DSpace · Aug 25, 2023 · da13913 · da13913
2 parents 25febb1 + 9340d4b
commit da13913
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 22 deletions.
diff --git a/src/app/shared/theme-support/theme.service.ts b/src/app/shared/theme-support/theme.service.ts
@@ -29,7 +29,7 @@ export const themeStateSelector = createFeatureSelector<ThemeState>('theme');
 
 export const currentThemeSelector = createSelector(
   themeStateSelector,
-  (state: ThemeState): string => hasValue(state) ? state.currentTheme : undefined
+  (state: ThemeState): string => hasValue(state) ? state.currentTheme : BASE_THEME_NAME,
 );
 
 @Injectable({
@@ -240,14 +240,7 @@ export class ThemeService {
       if (hasValue(parentThemeName)) {
         // inherit the head tags of the parent theme
         return this.createHeadTags(parentThemeName);
-      }
-      const defaultThemeConfig = getDefaultThemeConfig();
-      const defaultThemeName = defaultThemeConfig.name;
-      if (
-        hasNoValue(defaultThemeName) ||
-        themeName === defaultThemeName ||
-        themeName === BASE_THEME_NAME
-      ) {
+      } else {
         // last resort, use fallback favicon.ico
         return [
           this.createHeadTag({
@@ -260,9 +253,6 @@ export class ThemeService {
           })
         ];
       }
-
-      // inherit the head tags of the default theme
-      return this.createHeadTags(defaultThemeConfig.name);
     }
 
     return headTagConfigs.map(this.createHeadTag.bind(this));
@@ -425,9 +415,10 @@ export class ThemeService {
    * @private
    */
   private getActionForMatch(newTheme: Theme, currentThemeName: string): SetThemeAction | NoOpAction {
-    if (hasValue(newTheme) && newTheme.config.name !== currentThemeName) {
+    const newThemeName: string = newTheme?.config.name ?? BASE_THEME_NAME;
+    if (newThemeName !== currentThemeName) {
       // If we have a match, and it isn't already the active theme, set it as the new theme
-      return new SetThemeAction(newTheme.config.name);
+      return new SetThemeAction(newThemeName);
     } else {
       // Otherwise, do nothing
       return new NoOpAction();

diff --git a/src/app/shared/theme-support/themed.component.ts b/src/app/shared/theme-support/themed.component.ts
@@ -1,10 +1,10 @@
 import {
+  AfterViewInit,
   Component,
   ViewChild,
   ViewContainerRef,
   ComponentRef,
   SimpleChanges,
-  OnInit,
   OnDestroy,
   ComponentFactoryResolver,
   ChangeDetectorRef,
@@ -24,7 +24,7 @@ import { BASE_THEME_NAME } from './theme.constants';
   styleUrls: ['./themed.component.scss'],
   templateUrl: './themed.component.html',
 })
-export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges {
+export abstract class ThemedComponent<T> implements AfterViewInit, OnDestroy, OnChanges {
   @ViewChild('vcr', { read: ViewContainerRef }) vcr: ViewContainerRef;
   @ViewChild('content') themedElementContent: ElementRef;
   protected compRef: ComponentRef<T>;
@@ -74,8 +74,7 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
     }
   }
 
-  ngOnInit(): void {
-    this.destroyComponentInstance();
+  ngAfterViewInit(): void {
     this.initComponentInstance();
   }
 
@@ -96,8 +95,6 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
     }
 
     if (hasNoValue(this.lazyLoadObs)) {
-      this.destroyComponentInstance();
-
       this.lazyLoadObs = combineLatest([
         observableOf(changes),
         this.resolveThemedComponent(this.themeService.getThemeName()).pipe(
@@ -120,6 +117,7 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
     }
 
     this.lazyLoadSub = this.lazyLoadObs.subscribe(([simpleChanges, constructor]: [SimpleChanges, GenericConstructor<T>]) => {
+      this.destroyComponentInstance();
       const factory = this.resolver.resolveComponentFactory(constructor);
       this.compRef = this.vcr.createComponent(factory, undefined, undefined, [this.themedElementContent.nativeElement.childNodes]);
       if (hasValue(simpleChanges)) {

diff --git a/src/config/config.util.ts b/src/config/config.util.ts
@@ -5,7 +5,8 @@ import { environment } from '../environments/environment';
 import { hasNoValue } from '../app/shared/empty.util';
 
 import { AppConfig } from './app-config.interface';
-import { ThemeConfig } from './theme.model';
+import { ThemeConfig, NamedThemeConfig } from './theme.model';
+import { BASE_THEME_NAME } from '../app/shared/theme-support/theme.constants';
 
 /**
  * Extend Angular environment with app config.
@@ -44,7 +45,9 @@ const getDefaultThemeConfig = (): ThemeConfig => {
     hasNoValue(themeConfig.regex) &&
     hasNoValue(themeConfig.handle) &&
     hasNoValue(themeConfig.uuid)
-  );
+  ) ?? {
+    name: BASE_THEME_NAME,
+  } as NamedThemeConfig;
 };
 
 export {