Skip to content

When Openweathermap calls for multiple images, split icon Id and get multiple images #526

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.

Sign up for GitHub

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

Open
TomNaughton wants to merge 1 commit into
base: main
Choose a base branch
from
Open

When Openweathermap calls for multiple images, split icon Id and get multiple images #526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions immichFrame.Web/src/lib/components/elements/clock.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,14 @@
id="clockweatherinfo"
class="text-xl sm:text-xl md:text-2xl lg:text-3xl font-semibold text-shadow-sm weather-info"
>
{#if $configStore.weatherIconUrl }
<img src="{ $configStore.weatherIconUrl.replace('{IconId}', encodeURIComponent(weather.iconId)) }" class="icon-weather" alt="{weather.description}">
{#if $configStore.weatherIconUrl}
{#each weather.iconId?.split(',') ?? [] as icon (icon)}
Copy link
Collaborator

@3rob3 3rob3 Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this and it looks OK? I feel like it would look weird having two weather icons. My vote would be split iconId on comma, and just always use element [0].

Copy link
Collaborator

@JW-CH JW-CH Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same. Also, if the weather.iconId would be an empty string, the split would still return a single element (empty) which would request the URL with an empty id.

Copy link
Author

@TomNaughton TomNaughton Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered displaying a single element. This may work fine, but it may result in discounting a weather status that the user would prefer to know about. If the API returns "mist,heavy rain", the second element should probably take priority.

<img
src={$configStore.weatherIconUrl.replace('{IconId}', encodeURIComponent(icon.trim()))}
class="icon-weather"
alt={weather.description}
/>
{/each}
Comment on lines +78 to +85
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Normalize and filter icon IDs to avoid empty/duplicate entries, and consider precomputing the icon list

The multi‑icon rendering logic looks correct overall, but there are a couple of minor robustness concerns:

  • weather.iconId?.split(',') can produce empty strings (e.g., trailing comma or accidental double comma). Those will still render an <img> with an empty/invalid {IconId}, which can show as a broken image.
  • Using (icon) as the key can collide if the same icon ID appears more than once or differs only by whitespace.

You can tighten this up by normalizing and filtering the icon IDs once in a derived value and then iterating over that, using a stable key:

-				{#if $configStore.weatherIconUrl}
-					{#each weather.iconId?.split(',') ?? [] as icon (icon)}
-						<img
-							src={$configStore.weatherIconUrl.replace('{IconId}', encodeURIComponent(icon.trim()))}
-							class="icon-weather"
-							alt={weather.description}
-						/>
-					{/each}
-				{/if}
+				{#if $configStore.weatherIconUrl}
+					{#each weatherIcons() as icon, index (icon + '-' + index)}
+						<img
+							src={$configStore.weatherIconUrl.replace('{IconId}', encodeURIComponent(icon))}
+							class="icon-weather"
+							alt={weather.description}
+						/>
+					{/each}
+				{/if}

And in the <script> block, add a derived helper for the normalized list:

const weatherIcons = $derived(
	() =>
		weather?.iconId
			?.split(',')
			.map((id) => id.trim())
			.filter((id) => id.length > 0) ?? []
);

This avoids broken images from empty IDs, gives you predictable keys, and keeps the template a bit cleaner.

🤖 Prompt for AI Agents 
In immichFrame.Web/src/lib/components/elements/clock.svelte around lines 78–85,
the template splits weather.iconId inline which can yield empty strings and
duplicate keys; add a precomputed helper (e.g., weatherIcons) in the <script>
that: splits on commas, trims each id, filters out empty strings, and optionally
deduplicates, then iterate over that list in the template instead of
weather.iconId?.split(...), using a stable key such as `${icon}-${index}` and
encodeURIComponent(icon.trim()) for the src to avoid broken images and key
collisions.

{/if}
<div class="weather-location">{weather.location},</div>
<div class="weather-temperature">{weather.temperature?.toFixed(1)}</div>
Expand Down
Loading