feat(edge-apps-library): add default Screenly logo fallback to brand-logo

[nicomiguelino]

User description

Summary

• Add screenly.svg to edge-apps-library assets
• Return the default Screenly logo from setupBrandingLogo() when screenly_logo_light/screenly_logo_dark are not set or all fetch attempts fail
• Simplify setupBranding() since logoUrl is now always defined

---

PR Type

Bug fix, Enhancement, Tests

---

Description

• Add default Screenly logo fallback

• Normalize theme-based logo selection

• Always return logoUrl in branding

• Add fallback logo behavior tests

---

Diagram Walkthrough

flowchart LR
  settings["Theme and logo settings"]
  resolver["Resolve primary or secondary logo"]
  fetch["Fetch via proxy, then direct URL"]
  fallback["Use bundled `screenly.svg` fallback"]
  branding["Return branding config with `logoUrl`"]

  settings -- "selects inputs for" --> resolver
  resolver -- "provides URL to" --> fetch
  fetch -- "fails or missing config" --> fallback
  fetch -- "returns resolved logo to" --> branding
  fallback -- "supplies default to" --> branding

Loading

File Walkthrough

	Relevant files
Bug fix	theme.ts Default logo fallback for branding setup                                  edge-apps/edge-apps-library/src/utils/theme.ts Import bundled screenly.svg as default logo Return default logo when no branding logos exist Simplify logo resolution using primary/secondary selection Always return logoUrl from setupBranding() +16/-18
Tests	theme.test.ts Tests for branding logo fallback behavior                                edge-apps/edge-apps-library/src/utils/theme.test.ts Import setupBrandingLogo and bundled default logo Add test for missing configured logos Mock fetch failure to verify default fallback Preserve and restore global fetch between tests +35/-1

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e3a6676)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Packaging Risk Validate that the imported screenly.svg fallback is emitted and resolved correctly in the published library and in downstream consumers, otherwise the new fallback path may return a broken asset URL instead of an actual image. import defaultLogoUrl from '../assets/images/screenly.svg' API Contract setupBranding() now always includes logoUrl. Confirm that callers, serializers, and UI logic do not depend on logoUrl being omitted or undefined when no custom branding is configured. return { colors, logoUrl,

[github-actions]

Persistent review updated to latest commit 3cfa7ed

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 3cfa7ed
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Skip empty fallback fetches Avoid calling fetchLogoImage with an empty fallbackUrl, which can trigger a fetch to an unintended URL and noisy warnings. If fallbackUrl is falsy, return the default logo directly. edge-apps/edge-apps-library/src/utils/theme.ts [213-222] +if (!fallbackUrl) return defaultLogoUrl + try { return await fetchLogoImage(fallbackUrl) } catch (err) { console.warn( 'Failed to fetch logo image from fallback URL:', fallbackUrl, err, ) } // Return default logo if all fetches fail return defaultLogoUrl Suggestion importance[1-10]: 6 __ Why: This correctly prevents calling fetchLogoImage with an empty fallbackUrl, avoiding noisy warnings and unintended fetch behavior while keeping the new default-logo fallback semantics intact.	Low
	Make SVG fallback URL-safe Ensure the default SVG resolves to a URL string across build setups (some toolchains import SVGs as modules/components, not a string). Construct the asset URL via new URL(..., import.meta.url) to guarantee setupBrandingLogo() returns a string URL. edge-apps/edge-apps-library/src/utils/theme.ts [2] -import defaultLogoUrl from '../assets/images/screenly.svg' +const defaultLogoUrl = new URL( + '../assets/images/screenly.svg', + import.meta.url, +).toString() Suggestion importance[1-10]: 4 __ Why: Using new URL(..., import.meta.url).toString() can improve portability across toolchains where SVG imports aren’t plain strings, but it’s somewhat build-system-dependent and may not be necessary if the current bundler already provides a URL string for screenly.svg.	Low

---

Previous suggestions

Suggestions up to commit 3cfa7ed

Category	Suggestion	Impact
General	Preserve optional logo semantics Returning defaultLogoUrl through setupBranding() changes prior semantics where logoUrl could be omitted, which may be used to detect "custom branding" elsewhere. Preserve compatibility by only setting logoUrl when it’s not the default fallback. edge-apps/edge-apps-library/src/utils/theme.ts [232-235] return { colors, - logoUrl, + logoUrl: logoUrl === defaultLogoUrl ? undefined : logoUrl, } Suggestion importance[1-10]: 4 __ Why: This correctly identifies a potential behavior change: setupBranding() now always returns a logoUrl value, whereas previously it could be omitted via logoUrl: logoUrl || undefined. However, it’s speculative whether consumers rely on “missing logoUrl means unbranded,” and the PR may intentionally standardize on always returning a default, so the impact is moderate.	Low

[Copilot]

Pull request overview

Adds a built-in Screenly logo asset and updates branding utilities so brand-logo can fall back to a default logo when no custom logos are configured or when logo fetch attempts fail.

Changes:

• Add screenly.svg to the library’s image assets.
• Update setupBrandingLogo() to return the default logo when no logo is configured or when all fetch attempts fail.
• Simplify setupBranding() to always return a defined logoUrl.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File	Description
edge-apps/edge-apps-library/src/utils/theme.ts	Imports the default SVG and updates branding logo fallback + return shape.
edge-apps/edge-apps-library/src/assets/images/screenly.svg	Adds the default Screenly SVG logo asset.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Adds a built-in Screenly logo asset and uses it as a guaranteed fallback for branding logo resolution, so consumers (notably the brand-logo component) always have a usable logoUrl.

Changes:

• Added screenly.svg to library assets and imported it as the default logo fallback.
• Updated setupBrandingLogo() to return the default logo when no custom logo is configured or when all fetch attempts fail.
• Simplified setupBranding() to always return a defined logoUrl, and added unit tests for the new fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
edge-apps/edge-apps-library/src/utils/theme.ts	Imports the default Screenly logo and updates branding-logo fallback behavior / return shape.
edge-apps/edge-apps-library/src/utils/theme.test.ts	Adds coverage for default-logo fallback cases in setupBrandingLogo().
edge-apps/edge-apps-library/src/assets/images/screenly.svg	New default logo asset used as the fallback branding logo.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Persistent review updated to latest commit e3a6676

[github-actions]

PR Code Suggestions ✨

No code suggestions found for the PR.