feat(welcome-app): redesign and migrate to vanilla HTML/CSS/TS

[nicomiguelino]

Summary

• Migrates the Welcome App from Vue/Pinia to vanilla HTML/CSS/TypeScript
• Uses auto-scaler and app-header web components from @screenly/edge-apps
• Ports manifest files (screenly.yml, screenly_qc.yml) from the original app

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1cce6ae)

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Flakiness The module-level setupScreenlyMock() done before importing ./main.ts plus the DOMContentLoaded listener in main.ts can lead to init() being invoked implicitly (via the event listener) and explicitly (via direct init() calls in tests). This can make tests order-dependent or flaky depending on how Bun/JSDOM handles DOMContentLoaded at import time. Consider structuring main.ts so the DOMContentLoaded wiring is in a separate entrypoint, or ensure tests don’t trigger the listener (or assert idempotency). // Must be called before importing main.ts so that the global `screenly` object // is defined before the DOMContentLoaded listener fires during module load. setupScreenlyMock( {}, { welcome_heading: 'Welcome', welcome_message: 'to the team' }, ) import init from './main.ts' describe('Welcome App', () => { beforeEach(() => { document.body.innerHTML = ` <p id="welcome-heading"></p> <p id="welcome-message"></p> ` setupScreenlyMock( {}, { welcome_heading: 'Welcome', welcome_message: 'to the team', }, ) }) afterEach(() => { resetScreenlyMock() document.body.innerHTML = '' }) test('renders welcome heading from settings', () => { init() expect(document.querySelector('#welcome-heading')?.textContent).toBe( 'Welcome', ) Resource Cleanup The Playwright test closes the browser context only at the end of the happy path. If any step throws (navigation, load-state, screenshot), the context may remain open and cause cascading failures/leaks across resolutions. Wrapping the body in try/finally (closing context in finally) would make the suite more robust. for (const { width, height } of RESOLUTIONS) { test(`screenshot ${width}x${height}`, async ({ browser }) => { const screenshotsDir = getScreenshotsDir() const context = await browser.newContext({ viewport: { width, height } }) const page = await context.newPage() await setupClockMock(page) await setupScreenlyJsMock(page, screenlyJsContent) await page.goto('/') await page.waitForLoadState('networkidle') await page.screenshot({ path: path.join(screenshotsDir, `${width}x${height}.png`), fullPage: false, }) await context.close() }) Asset Path Risk Using an absolute URL for the background image (/static/images/bg.webp) assumes the app is always served from the domain root. If the deployed app is hosted under a sub-path (or uses a non-root base), the background may 404. Consider using a relative path or bundler-friendly URL resolution so assets work regardless of base path. body { margin: 0; padding: 0; overflow: hidden; color: white; font-family: 'Inter', system-ui, sans-serif; background: url('/static/images/bg.webp') no-repeat center center; background-size: cover; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1cce6ae
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Make screenshots deterministic and leak-free Avoid networkidle for screenshot readiness since it can hang or be flaky when long-lived requests exist. Also ensure the browser context is closed even if navigation/screenshotting throws to prevent leaked contexts across tests. Wait for a stable UI signal (e.g., a selector) and close in a finally. edge-apps/welcome-app-new/e2e/screenshots.spec.ts [27-41] const context = await browser.newContext({ viewport: { width, height } }) const page = await context.newPage() -await setupClockMock(page) -await setupScreenlyJsMock(page, screenlyJsContent) +try { + await setupClockMock(page) + await setupScreenlyJsMock(page, screenlyJsContent) -await page.goto('/') -await page.waitForLoadState('networkidle') + await page.goto('/') + await page.waitForSelector('#welcome-heading') -await page.screenshot({ - path: path.join(screenshotsDir, `${width}x${height}.png`), - fullPage: false, -}) + await page.screenshot({ + path: path.join(screenshotsDir, `${width}x${height}.png`), + fullPage: false, + }) +} finally { + await context.close() +} -await context.close() - Suggestion importance[1-10]: 7 __ Why: Replacing page.waitForLoadState('networkidle') with a deterministic UI wait (e.g., #welcome-heading) reduces flakiness/hangs in Playwright tests. Wrapping the flow in try/finally ensures context.close() always runs, preventing leaked contexts across iterations/tests.	Medium
	Initialize even after DOM ready Ensure the app still initializes when the module is loaded after DOMContentLoaded has already fired (e.g., dynamic import or certain test runners). Without this, the UI may never render and ready_signal may never be sent. Gate initialization on document.readyState and run immediately when the DOM is already ready. edge-apps/welcome-app-new/src/main.ts [35-39] -document.addEventListener('DOMContentLoaded', () => { +const bootstrap = () => { setupErrorHandling() setupTheme() init() -}) +} +if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', bootstrap, { once: true }) +} else { + bootstrap() +} + Suggestion importance[1-10]: 5 __ Why: The change makes initialization more robust if main.ts is loaded after DOMContentLoaded (e.g., dynamic import or some runners), preventing missed UI render/signalReady(). In this app’s current setup (module script at end of body), it’s unlikely to be an issue, so impact is moderate.	Low

---

Previous suggestions

Suggestions up to commit 7b72f8a

Category	Suggestion	Impact
Possible issue	Make screenshots wait deterministic Avoid networkidle here since it can be flaky (e.g., long-polling/analytics) and make screenshot tests hang. Also ensure the browser context is always closed by using try/finally, otherwise failures before context.close() can leak resources and cascade failures across resolutions. edge-apps/welcome-app-new/e2e/screenshots.spec.ts [27-41] const context = await browser.newContext({ viewport: { width, height } }) -const page = await context.newPage() +try { + const page = await context.newPage() -await setupClockMock(page) -await setupScreenlyJsMock(page, screenlyJsContent) + await setupClockMock(page) + await setupScreenlyJsMock(page, screenlyJsContent) -await page.goto('/') -await page.waitForLoadState('networkidle') + await page.goto('/') + await page.waitForFunction(() => { + const h = document.querySelector('#welcome-heading')?.textContent?.trim() ?? '' + const m = document.querySelector('#welcome-message')?.textContent?.trim() ?? '' + return h.length > 0 && m.length > 0 + }) + await page.evaluate(() => (document as any).fonts?.ready) -await page.screenshot({ - path: path.join(screenshotsDir, `${width}x${height}.png`), - fullPage: false, -}) + await page.screenshot({ + path: path.join(screenshotsDir, `${width}x${height}.png`), + fullPage: false, + }) +} finally { + await context.close() +} -await context.close() - Suggestion importance[1-10]: 7 __ Why: Replacing page.waitForLoadState('networkidle') with a deterministic readiness check reduces flakiness/hanging risk in screenshot tests, and wrapping the context in try/finally ensures context.close() runs even on failure. This is a solid reliability improvement for the e2e suite.	Medium
	Initialize error handling earlier Call setupErrorHandling() at module load time so it can catch errors thrown before DOMContentLoaded (e.g., during component registration or early script execution). Keeping it inside the event listener can miss critical initialization failures and prevent reporting. edge-apps/welcome-app-new/src/main.ts [9-34] +setupErrorHandling() + document.addEventListener('DOMContentLoaded', () => { - setupErrorHandling() - const welcomeHeading = getSettingWithDefault<string>( 'welcome_heading', 'Welcome', ) const welcomeMessage = getSettingWithDefault<string>( 'welcome_message', 'to the team', ) const headingEl = document.querySelector<HTMLParagraphElement>('#welcome-heading') if (headingEl) { headingEl.textContent = welcomeHeading } const messageEl = document.querySelector<HTMLParagraphElement>('#welcome-message') if (messageEl) { messageEl.textContent = welcomeMessage } signalReady() }) Suggestion importance[1-10]: 4 __ Why: Moving setupErrorHandling() outside the DOMContentLoaded handler can catch runtime errors that occur after module evaluation but before the DOM event fires, which is a reasonable robustness improvement. However, it still won’t catch failures during the earlier import/module initialization phase, so the practical impact is limited.	Low

[Copilot]

Pull request overview

Migrates the Welcome App implementation to a vanilla HTML/CSS/TypeScript app using shared @screenly/edge-apps web components and tooling, and adds manifests + screenshot artifacts for distribution/validation.

Changes:

• Add vanilla app entrypoint (index.html + src/main.ts) and styling (src/css/style.css) using auto-scaler and app-header.
• Add Playwright-based screenshot generation test and commit generated screenshots.
• Add screenly.yml / screenly_qc.yml, package scripts, and project docs/ignores.

Reviewed changes

Copilot reviewed 10 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
edge-apps/welcome-app-new/src/main.ts	Initializes app, reads settings, writes welcome text, signals ready
edge-apps/welcome-app-new/src/css/style.css	Provides layout/typography styling for redesigned welcome screen
edge-apps/welcome-app-new/index.html	Defines app shell using auto-scaler + app-header
edge-apps/welcome-app-new/e2e/screenshots.spec.ts	Generates screenshots across supported resolutions
edge-apps/welcome-app-new/screenshots/*.webp	Screenshot artifacts for supported resolutions
edge-apps/welcome-app-new/screenly.yml	Production manifest for app + settings
edge-apps/welcome-app-new/screenly_qc.yml	QC manifest for app + settings
edge-apps/welcome-app-new/package.json	Adds build/dev/lint/type-check/screenshot scripts and deps
edge-apps/welcome-app-new/README.md	Setup, deploy, configuration, and workflow docs
edge-apps/welcome-app-new/.ignore	Packaging ignore list
edge-apps/welcome-app-new/.gitignore	Git ignore list

---

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

[github-actions]

Persistent review updated to latest commit 1cce6ae

[Copilot]

Pull request overview

Copilot reviewed 11 out of 23 changed files in this pull request and generated no new comments.

---

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