Image assets (3/3): widget config UI + settings image-cache card (replicates mxtommy/Kip #1082)

[mairas]

Replicates upstream PR mxtommy/Kip#1082 ("Image assets (3/3): widget config UI + settings image-cache card") by dillan.

This is the third part of the image-assets series: the Image widget, its image-source-setup config UI (upload, gallery, fit/background), the ImageAssetService + shared plugin-URL helper, and an image-cache card surfaced under a new Media settings tab.

Method: commit-by-commit cherry-pick of the upstream PR range (linear history, no merge commits). One commit (d3b07875, "Use takeUntilDestroyed on image-cache subscriptions") became empty after conflict resolution and was skipped, because its only change targeted the image-cache code that SKip keeps on its own side of config.component (see below).

⚠️ Conflict resolution applied

The upstream PR first adds an image-cache card directly into config.component, then a later commit (adf95687) moves that card out into a brand-new media.component Media tab. SKip's config.component is independently diverged (rewritten around profiles), so the intermediate "add card to config" states conflicted. Because the PR's net effect on config.component is zero (the card lives in the new Media tab), every conflict there was resolved by keeping SKip's version:

• src/app/core/components/options/configuration/config.component.ts — kept SKip's profiles-based version; the cache-card logic lands in the new media.component instead.
• src/app/core/components/options/configuration/config.component.html — kept SKip's version (no cache card in config).
• src/app/core/components/options/configuration/config.component.scss — kept SKip's version (verified byte-identical to base).
• src/app/core/components/options/configuration/config.component.spec.ts — kept SKip's version.
• CHANGELOG.md — kept SKip's changelog; the upstream entry was not merged.

The new media.component, the Media tab wiring in tabs.component, the ImageAssetService, the Image widget, and the image-source-setup config UI were all applied unchanged from upstream. The resulting net diff is identical to the upstream PR except for the intentionally-preserved SKip CHANGELOG.md.

Needs a build + review before merge.

Note

This is an experimental replica carried in the fork for evaluation. No build/install/lint was run during replication; package-lock.json was not affected (the PR adds no dependencies).

[mairas]

Content review: accept-with-fixes — clean, idiomatic client; one real reactivity bug

Read the widget, config sub-component, ImageAssetService, and URL util across security and correctness lenses.

Genuinely solid (verified):

• Client-security is clean — I couldn't construct a script-execution path. Every untrusted server field (asset name, error strings, cache stats) lands in an auto-escaped {{ }}/attribute context; no [innerHTML], bypassSecurityTrust*, DomSanitizer, [href], or srcdoc. The server-supplied image URL field is deliberately ignored and ids are re-encoded before use.
• Subscription hygiene is complete: every subscription (service endpoint, upload, delete dialog+delete, gallery refresh, cache stats, purge, modal statusChanges) uses takeUntilDestroyed; the gallery-refresh race is handled with a monotonic galleryRequestSeq that drops stale responses.

Findings:

[medium · correctness] Image widget can stay blank after cold boot — widget-image.component.ts:47-51 + image-asset.service.ts. imageUrl is a computed() tracking imageConfig()/containerWidth(), but it calls images.urlFor(id,w) which reads the plain, non-signal pluginBaseUrl, populated asynchronously in the service constructor's serverServiceEndpoint$ subscription. If the ResizeObserver fires before the endpoint resolves (cold boot), the computed pins and never re-evaluates when pluginBaseUrl becomes valid → the widget shows empty state indefinitely until the crew resizes/re-opens it. Make pluginBaseUrl a signal so the computed re-runs.

[medium → low · marine-safety] "No image selected" shown when an image IS configured but not yet loadable — the else-branch conflates "unconfigured" with "configured but urlFor returned null because pluginBaseUrl isn't ready." On a safety display, telling the operator nothing is configured when a stowage/through-hull diagram is configured is misleading. Verified real. Distinguish the two states.

Plus low: an initial 0-width container snaps to the 2560 px variant → a full-res fetch then an immediate re-fetch at the real size.

Bottom line: well-built, idiomatic zoneless-Angular with correct subscription and race handling; the reactivity bug is the real one (widget blank after boot until interaction). Adopt together with #54 — this client is inert without the server plugin.

(Resolution applied earlier: Image widget → main's lazy registry, and broke a DialogService import cycle the merge exposed by opening the confirmation dialog via MatDialog directly. Full vitest suite 576/576; CI green.)

[mairas]

Fixes pushed

Addresses the correctness + empty-state findings:

• Blank-after-boot (reactivity): ImageAssetService.pluginBaseUrl is now a signal, so the widget's imageUrl computed re-runs once the SK endpoint resolves after first paint, instead of staying blank until resize/reopen.
• Empty-state: the widget distinguishes "no image configured" from "configured but not yet loadable" — the latter shows "Loading image…" rather than the misleading "No image selected".
• 0-width variant: snapImageWidth snaps an unknown/zero width to the smallest variant, not the largest, so the initial render no longer fetches the full-res image then immediately re-fetches.

Tests: reactive recompute + loading state (widget), reactive base URL (service), smallest-for-zero-width (util). Full vitest suite 579/579; CI green.