Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Update theme switcher to use Angular 18 practices #9250

Merged
merged 40 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
76c0243
Update theme components to use signals
FelixTJDietrich Aug 26, 2024
2df316c
move variables around
FelixTJDietrich Aug 26, 2024
33c849e
move more variables
FelixTJDietrich Aug 26, 2024
ccaef4a
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Aug 27, 2024
92d26d6
try fixing tests
FelixTJDietrich Aug 27, 2024
639bda7
try fixing tests
FelixTJDietrich Aug 29, 2024
cb5468f
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Aug 30, 2024
5ddeab5
fix theme service tests
FelixTJDietrich Aug 30, 2024
9da056e
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Sep 10, 2024
f5ad47e
Merge remote-tracking branch 'origin/chore/performance/angular-18-sig…
FelixTJDietrich Sep 12, 2024
7ed3cbc
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Sep 12, 2024
ad43212
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Sep 12, 2024
aca687e
remove effect and replace with RxJS
FelixTJDietrich Sep 12, 2024
d3b2920
fix system preference
FelixTJDietrich Sep 12, 2024
b40ef0c
fix some tests
FelixTJDietrich Sep 12, 2024
378f514
fix naming
FelixTJDietrich Sep 12, 2024
19bd309
try fixing test issues
FelixTJDietrich Sep 12, 2024
78010e0
fix more tests and simplify emoji stuff
FelixTJDietrich Sep 13, 2024
ea144b3
fix theme-switch.component.spec.ts
JohannesWt Sep 18, 2024
3438c37
change signal logic slightly & update client tests
JohannesWt Oct 7, 2024
8ce0858
fix more client tests
JohannesWt Oct 7, 2024
7d78fdd
fix more client tests
JohannesWt Oct 7, 2024
1fd68f4
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
JohannesWt Oct 8, 2024
7b76e35
fix monaco-editor.service.spec.ts
JohannesWt Oct 8, 2024
ccc07c3
fix code-editor-instructor.integration.spec.ts
JohannesWt Oct 8, 2024
2a19411
add track expression & add fix more programming-exercise-instruction.…
JohannesWt Oct 8, 2024
d99223c
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 7, 2024
c333b95
fix tests
FelixTJDietrich Nov 7, 2024
e5add94
fix programming exercise instructions component
FelixTJDietrich Nov 8, 2024
c4cd5fa
fix translation
FelixTJDietrich Nov 8, 2024
0740832
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 8, 2024
d4c0335
Merge branch 'chore/performance/angular-18-signal-migration-example' …
FelixTJDietrich Nov 8, 2024
e697bb5
fix modules?
FelixTJDietrich Nov 8, 2024
3320c6c
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 13, 2024
a2abbdd
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 13, 2024
2d4f970
fix failing tests
FelixTJDietrich Nov 13, 2024
835f676
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 13, 2024
605c833
Merge branch 'chore/performance/angular-18-signal-migration-example' …
FelixTJDietrich Nov 13, 2024
cfacf8b
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
FelixTJDietrich Nov 13, 2024
7a4ff5d
Merge branch 'develop' into chore/performance/angular-18-signal-migra…
krusche Nov 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/dev/guidelines/client-design.rst
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ Example:

.. code-block:: ts

this.themeService.applyThemeExplicitly(Theme.DARK);
this.themeService.applyThemePreference(Theme.DARK);
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved



4 changes: 2 additions & 2 deletions src/main/webapp/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { OrionOutdatedComponent } from 'app/shared/orion/outdated-plugin-warning
import { LoadingNotificationComponent } from 'app/shared/notification/loading-notification/loading-notification.component';
import { NotificationPopupComponent } from 'app/shared/notification/notification-popup/notification-popup.component';
import { UserSettingsModule } from 'app/shared/user-settings/user-settings.module';
import { ThemeModule } from 'app/core/theme/theme.module';
import { ThemeSwitchComponent } from 'app/core/theme/theme-switch.component';
import { ArtemisSharedComponentModule } from 'app/shared/components/shared-component.module';
import { FaIconLibrary } from '@fortawesome/angular-fontawesome';
import { artemisIconPack } from 'src/main/webapp/content/icons/icons';
Expand All @@ -42,7 +42,7 @@ import { ScrollingModule } from '@angular/cdk/scrolling';
ArtemisComplaintsModule,
ArtemisHeaderExercisePageWithDetailsModule,
UserSettingsModule,
ThemeModule,
ThemeSwitchComponent,
ArtemisSharedComponentModule,
ScrollingModule,
],
Expand Down
10 changes: 5 additions & 5 deletions src/main/webapp/app/core/theme/theme-switch.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
<div class="popover-content" (mouseenter)="openPopover()" id="theme-switch-popover-content">
<div class="head">☾ <span jhiTranslate="artemisApp.theme.darkMode"></span> ☾</div>
<div class="switch-box">
<div><fa-icon [icon]="faSync" /> {{ 'artemisApp.theme.sync' | artemisTranslate }}</div>
<div><fa-icon [icon]="faSync" /> <span jhiTranslate="artemisApp.theme.sync"></span></div>
<div class="form-switch">
<input class="form-check-input" type="checkbox" (click)="toggleSynced()" [checked]="isSynced" />
<input class="form-check-input" type="checkbox" (click)="toggleSynced()" [checked]="isSyncedWithOS()" />
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
</div>
</div>
</div>
Expand All @@ -17,10 +17,10 @@
[triggers]="''"
#popover="ngbPopover"
[autoClose]="false"
[animation]="animate"
[placement]="popoverPlacement"
[animation]="true"
[placement]="popoverPlacement()"
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
>
<div class="theme-toggle" [ngClass]="{ dark: isDark }" id="theme-toggle">
<div class="theme-toggle" [class.dark]="isDarkTheme()" id="theme-toggle">
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
<svg class="sun-and-moon" aria-hidden="true" width="24" height="24" viewBox="0 0 24 24">
<circle class="sun" cx="12" cy="12" r="6" mask="url(#moon-mask)" fill="currentColor" />
<g class="sun-beams" stroke="currentColor">
Expand Down
57 changes: 23 additions & 34 deletions src/main/webapp/app/core/theme/theme-switch.component.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { Component, Input, OnInit, ViewChild } from '@angular/core';
import { NgbPopover } from '@ng-bootstrap/ng-bootstrap';
import { ChangeDetectionStrategy, Component, OnInit, computed, inject, input, viewChild } from '@angular/core';
import { CommonModule } from '@angular/common';
import { ArtemisSharedModule } from 'app/shared/shared.module';
import { TranslateModule } from '@ngx-translate/core';
import { NgbModule, NgbPopover } from '@ng-bootstrap/ng-bootstrap';
import { PlacementArray } from '@ng-bootstrap/ng-bootstrap/util/positioning';
import { Theme, ThemeService } from 'app/core/theme/theme.service';
import { fromEvent } from 'rxjs';
import { faSync } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeModule } from '@fortawesome/angular-fontawesome';

/**
* Displays a sun or a moon in the navbar, depending on the current theme.
Expand All @@ -13,56 +18,41 @@ import { faSync } from '@fortawesome/free-solid-svg-icons';
selector: 'jhi-theme-switch',
templateUrl: './theme-switch.component.html',
styleUrls: ['theme-switch.component.scss'],
imports: [TranslateModule, CommonModule, ArtemisSharedModule, NgbModule, FontAwesomeModule],
changeDetection: ChangeDetectionStrategy.OnPush,
standalone: true,
})
export class ThemeSwitchComponent implements OnInit {
@ViewChild('popover') popover: NgbPopover;
protected readonly faSync = faSync;

@Input() popoverPlacement: string;
private readonly themeService = inject(ThemeService);

isDark = false;
isSynced = false;
animate = true;
openPopupAfterNextChange = false;
closeTimeout: any;
popoverPlacement = input.required<PlacementArray>();
popover = viewChild.required<NgbPopover>('popover');

// Icons
faSync = faSync;
isDarkTheme = computed(() => this.themeService.currentTheme() === Theme.DARK);
isSyncedWithOS = computed(() => this.themeService.userPreference() === undefined);

constructor(private themeService: ThemeService) {}
closeTimeout: any;

ngOnInit() {
// Listen to theme changes to change our own state accordingly
this.themeService.getCurrentThemeObservable().subscribe((theme) => {
this.isDark = theme === Theme.DARK;
this.animate = true;
if (this.openPopupAfterNextChange) {
this.openPopupAfterNextChange = false;
setTimeout(() => this.openPopover(), 250);
}
});

// Listen to preference changes
this.themeService.getPreferenceObservable().subscribe((themeOrUndefined) => {
this.isSynced = !themeOrUndefined;
});

// Workaround as we can't dynamically change the "autoClose" property on popovers
fromEvent(window, 'click').subscribe((e) => {
const popoverContentElement = document.getElementById('theme-switch-popover-content');
if (this.popover.isOpen() && !popoverContentElement?.contains(e.target as Node)) {
if (this.popover().isOpen() && !popoverContentElement?.contains(e.target as Node)) {
this.closePopover();
}
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
});
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
}

openPopover() {
this.popover?.open();
this.popover().open();
clearTimeout(this.closeTimeout);
}

closePopover() {
clearTimeout(this.closeTimeout);
this.popover?.close();
this.popover().close();
}

mouseLeave() {
Expand All @@ -74,9 +64,8 @@ export class ThemeSwitchComponent implements OnInit {
* Changes the theme to the currently not active theme.
*/
toggleTheme() {
this.animate = false;
this.openPopupAfterNextChange = true;
setTimeout(() => this.themeService.applyThemeExplicitly(this.isDark ? Theme.LIGHT : Theme.DARK));
this.themeService.applyThemePreference(this.isDarkTheme() ? Theme.LIGHT : Theme.DARK);
setTimeout(() => this.openPopover(), 250);
}

/**
Expand All @@ -85,6 +74,6 @@ export class ThemeSwitchComponent implements OnInit {
* - if it's currently not synced, we remove the preference to apply the system theme
*/
toggleSynced() {
this.themeService.applyThemeExplicitly(this.isSynced ? this.themeService.getCurrentTheme() : undefined);
this.themeService.applyThemePreference(this.isSyncedWithOS() ? this.themeService.currentTheme() : undefined);
}
}
12 changes: 0 additions & 12 deletions src/main/webapp/app/core/theme/theme.module.ts

This file was deleted.

142 changes: 51 additions & 91 deletions src/main/webapp/app/core/theme/theme.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Injectable } from '@angular/core';
import { Injectable, computed, effect, inject, signal, untracked } from '@angular/core';
import { IconDefinition, faMoon, faSun } from '@fortawesome/free-solid-svg-icons';
import { LocalStorageService } from 'ngx-webstorage';
import { BehaviorSubject, Observable } from 'rxjs';

export const THEME_LOCAL_STORAGE_KEY = 'artemisapp.theme.preference';
export const THEME_OVERRIDE_ID = 'artemis-theme-override';
Expand Down Expand Up @@ -44,43 +43,37 @@ export class Theme {
})
export class ThemeService {
/**
* The currently applied theme
* The user preference changes as WritableSignal.
* If changed, the theme is applied immediately.
*/
private currentTheme: Theme = Theme.LIGHT;
/**
* A behavior subject that fires for each new applied theme.
*/
private currentThemeSubject: BehaviorSubject<Theme> = new BehaviorSubject<Theme>(Theme.LIGHT);
/**
* A behavior subject that fires if the user preference changes.
* Can be either a theme for an explicit theme or undefined if system settings are preferred
*/
private preferenceSubject: BehaviorSubject<Theme | undefined> = new BehaviorSubject<Theme | undefined>(undefined);

private darkSchemeMediaQuery: MediaQueryList;

constructor(private localStorageService: LocalStorageService) {}
private _userPreference = signal<Theme | undefined>(undefined);

FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
/**
* Returns the currently active theme.
* The user preference changes as ReadonlySignal.
* Can be either a theme for an explicit theme or undefined if system settings are preferred.
*/
public getCurrentTheme(): Theme {
return this.currentTheme;
}
public readonly userPreference = this._userPreference.asReadonly();

/**
* Returns an observable that will be fired immediately for the current theme and for each future theme change until unsubscribed.
* The system preference as WritableSignal.
*/
public getCurrentThemeObservable(): Observable<Theme> {
return this.currentThemeSubject.asObservable();
}
private systemPreference = signal<Theme>(Theme.LIGHT);

/**
* Returns an observable that will be fired immediately for the current user preference and if the user preference changes.
* Can be either a theme for an explicit theme or undefined if system settings are preferred
* The currently applied theme as Signal.
*/
public getPreferenceObservable(): Observable<Theme | undefined> {
return this.preferenceSubject.asObservable();
public currentTheme = computed(() => this.userPreference() ?? this.systemPreference());

private localStorageService = inject(LocalStorageService);

private darkSchemeMediaQuery: MediaQueryList;
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved

constructor() {
effect(() => {
// Apply the theme as soon as the currentTheme changes
const currentTheme = this.currentTheme();
untracked(() => this.applyTheme(currentTheme));
});
}

/**
Expand All @@ -91,52 +84,35 @@ export class ThemeService {
initialize() {
this.darkSchemeMediaQuery = window.matchMedia('(prefers-color-scheme: dark)');
if (this.darkSchemeMediaQuery.media !== 'not all') {
this.darkSchemeMediaQuery.addEventListener('change', () => this.applyPreferredTheme());
this.darkSchemeMediaQuery.addEventListener('change', () => {
if (this.darkSchemeMediaQuery.matches) {
this.systemPreference.set(Theme.DARK);
} else {
this.systemPreference.set(Theme.LIGHT);
}
});
}

addEventListener('storage', (event) => {
if (event.key === 'jhi-' + THEME_LOCAL_STORAGE_KEY) {
this.preferenceSubject.next(this.getStoredTheme());
this.applyPreferredTheme();
this._userPreference.set(this.getStoredThemePreference());
}
});

this.applyPreferredTheme();
this.preferenceSubject.next(this.getStoredTheme());
}

/**
* Applies the preferred theme.
* The preferred theme is either
* - the theme stored in local storage, if present, or else
* - the system preference, if present, or else
* - the default theme
*/
private applyPreferredTheme() {
const storedTheme = this.getStoredTheme();
if (storedTheme) {
this.applyThemeInternal(storedTheme);
return;
}

if (this.darkSchemeMediaQuery.matches) {
this.applyThemeInternal(Theme.DARK);
return;
}

this.applyThemeInternal(Theme.LIGHT);
this.systemPreference.set(this.darkSchemeMediaQuery.matches ? Theme.DARK : Theme.LIGHT);
this._userPreference.set(this.getStoredThemePreference());
}

/**
* Returns the theme preference stored in local storage or undefined if no preference is stored
*/
private getStoredTheme(): Theme | undefined {
private getStoredThemePreference(): Theme | undefined {
const storedIdentifier = this.localStorageService.retrieve(THEME_LOCAL_STORAGE_KEY);
const storedTheme = Theme.all.find((theme) => theme.identifier === storedIdentifier);

// An unknown theme was stored. Let's clear it
if (storedIdentifier && !storedTheme) {
this.storePreference(undefined);
this.localStorageService.clear(THEME_LOCAL_STORAGE_KEY);
}

return storedTheme;
Expand Down Expand Up @@ -170,37 +146,36 @@ export class ThemeService {
}

/**
* Applies the specified theme.
* Applies the specified theme preference.
* Should only be called upon user request.
* Stores the preference in local storage.
*
* @param theme the theme to be applied; pass undefined to use system preference mode
* @param preference the theme to be applied; pass undefined to use system preference mode
*/
public applyThemeExplicitly(theme: Theme | undefined) {
this.storePreference(theme);
this.applyPreferredTheme();
}

private applyThemeInternal(theme: Theme) {
if (!theme) {
return;
}

// Do not inject or remove anything from the DOM if the applied theme is the current theme
if (this.currentTheme === theme) {
return;
public applyThemePreference(preference: Theme | undefined) {
if (preference) {
this.localStorageService.store(THEME_LOCAL_STORAGE_KEY, preference.identifier);
} else {
this.localStorageService.clear(THEME_LOCAL_STORAGE_KEY);
}
this._userPreference.set(preference);
}

/**
* Applies the theme to the application.
*
* Only call if the theme changed.
*
* @param theme the theme to apply
*/
private applyTheme(theme: Theme) {
// Get current <link> theme override
const overrideTag = document.getElementById(THEME_OVERRIDE_ID);

if (theme.isDefault) {
// The default theme is always injected by Angular; therefore, we just need to remove
// our theme override, if present
overrideTag?.remove();

this.currentTheme = theme;
this.currentThemeSubject.next(theme);
} else {
// If the theme is not the default theme, we need to add a theme override stylesheet to the page header

Expand All @@ -215,11 +190,8 @@ export class ThemeService {
newTag.href = theme.fileName! + '?_=' + new Date().setMinutes(0, 0, 0);

// As soon as the new style sheet loaded, remove the old override (if present)
// and fire the subject to inform other services and components
newTag.onload = () => {
overrideTag?.remove();
this.currentTheme = theme;
this.currentThemeSubject.next(theme);
};

// Insert the new stylesheet link tag after the last existing link tag
Expand All @@ -229,18 +201,6 @@ export class ThemeService {
}
}

private storePreference(theme?: Theme) {
if (theme) {
this.localStorageService.store(THEME_LOCAL_STORAGE_KEY, theme.identifier);
} else {
this.localStorageService.clear(THEME_LOCAL_STORAGE_KEY);
}

if (this.preferenceSubject.getValue() !== theme) {
this.preferenceSubject.next(theme);
}
}

/**
* Hides the notification sidebar as there will be an overlay ove the whole page
* that covers details of the exam summary (=> exam summary cannot be read).
Expand Down
Loading
Loading