Fixed changing background image based on light/dark theme

[gastoner]

What does this PR do?

Fix changing background image based on light/dark theme

Screenshot / video of UI

Screen.Recording.2024-09-04.152535.mp4

What issues does this PR fix or reference?

Close #8526

How to test this PR?

Switch between light/dark theme and check if the recommendation banners reflect the change

[axel7083]

Screencast.from.2024-09-04.16-34-14.mp4

When trying to very quickly change the theme, I am able to have some opposite result

[axel7083]

The ExtensionBanners.svelte fetch the isDark on mount, and it is not refresh as the component is reused.

podman-desktop/packages/renderer/src/lib/recommendation/ExtensionBanners.svelte

Lines 11 to 15 in aba56f0

	onMount(async () => {
	// Retrieve the current theme appearance colour of PD for the banners we will be showing
	const appearanceUtil = new AppearanceUtil();
	isDark = await appearanceUtil.isDarkMode();
	});

[gastoner]

@axel7083 try it now!

[axel7083]

The ExtensionBanners.svelte fetch the isDark on mount, and it is not refresh as the component is reused.

podman-desktop/packages/renderer/src/lib/recommendation/ExtensionBanners.svelte

Lines 11 to 15 in aba56f0

	onMount(async () => {
	// Retrieve the current theme appearance colour of PD for the banners we will be showing
	const appearanceUtil = new AppearanceUtil();
	isDark = await appearanceUtil.isDarkMode();
	});

We might need to either, get the isDark at every render (possible but not really performance wise) or trying to get an event listener for it :?

[gastoner]

The ExtensionBanners.svelte fetch the isDark on mount, and it is not refresh as the component is reused.

podman-desktop/packages/renderer/src/lib/recommendation/ExtensionBanners.svelte

Lines 11 to 15 in aba56f0

	onMount(async () => {
	// Retrieve the current theme appearance colour of PD for the banners we will be showing
	const appearanceUtil = new AppearanceUtil();
	isDark = await appearanceUtil.isDarkMode();
	});

We might need to either, get the isDark at every render (possible but not really performance wise) or trying to get an event listener for it :?

The listener would be probably good, now it should get the boolean value each time user enters the dashboard

[benoitf]

it looks like ApperanceUtil isDark or getTheme should be removed and it should be a state (v5) or store v4 being updated/refreshed when configuration is changing

[axel7083]

it looks like ApperanceUtil isDark or getTheme should be removed and it should be a state (v5) or store v4 being updated/refreshed when configuration is changing

The ApperanceUtil is doing some hacky stuff to determine the theme

podman-desktop/packages/renderer/src/lib/appearance/appearance-util.ts

Lines 41 to 53 in 9c946fd

	async getTheme(): Promise<string> {
	const themeName = await window.getConfigurationValue<string>(
	AppearanceSettings.SectionName + '.' + AppearanceSettings.Appearance,
	);
	const systemTheme = window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light';
	if (themeName === AppearanceSettings.SystemEnumValue) {
	return systemTheme;
	}
	return themeName ?? systemTheme;
	}

Looking at the api of MediaQueryList there is a addEventListener and removeEventListener which would allow the base system for a quick hack.

But since we are using a configuration value (light / dark / system), we might want to deprecate the AppearanceUtil and have a store as suggested by @benoitf

[gastoner]

it looks like ApperanceUtil isDark or getTheme should be removed and it should be a state (v5) or store v4 being updated/refreshed when configuration is changing

Should I convert it to v5 then? I've done something like this 6db3c75

[gastoner]

@feloy This PR needs to be updated after will be #8759 closed, probably should convert it to draft for now

[feloy]

@feloy This PR needs to be updated after will be #8759 closed, probably should convert it to draft for now

sorry, my comment was for #8759

[gastoner]

There would be a need to improve system change detection (similar problem in MonacoEditor)

[feloy]

As far as I understand svelte reactivity, it seems to me that the changes in this file are not necessary, we are just adding a layer of reactivity.
(and testing without these changes, it works the same)

[gastoner]

I see, thank you, I thought that I've forgot to use subscribe here

[benoitf]

nice to see using also the new svelte5 syntax