Overhaul samples app architecture and CI - #1253
Merged
Merged
Conversation
The nav sidebar was server-rendered into all 962 pages (~185MB of dist), Rollup hosted shared library code inside arbitrary sample chunks (11MB chunks, wrong-theme leaks), samples flashed unstyled until their JS chunk evaluated, and CI only built. - Render the nav on the client from a static nav.json endpoint; delete NavSidebar.astro. Sidebar visibility is decided before first paint, so no layout shift. - Split node_modules into per-package vendor chunks (plus a chunk for Vite's preload helper) so no sample chunk hosts shared code and unrelated pages never run a foreign sample's side effects. - Emit each sample's stylesheets into <head> at build time: theme sheets as cacheable <link>s copied to public/ig-themes/, local CSS/SCSS inlined. Fixes first-paint flash and the memoised getTheme() latching onto another sample's theme. - Extract the triplicated glob→slug pipeline into samples.ts. - Gate CI with astro check and Playwright smoke tests (14 tests against the production build); retire the broken Azure pipeline.
ChronosSF
previously approved these changes
Sep 2, 2026
There was a problem hiding this comment.
🟡 Changes recommended
Navigation timing, sandboxed iframe storage access, and overly broad smoke-test error suppression remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Overhauls the Astro samples browser to reduce duplicated output, prevent cross-sample styling issues, and strengthen CI.
Changes:
- Moves navigation rendering to a client-fetched static tree.
- Emits sample styles in
<head>and isolates shared dependencies into vendor chunks. - Adds type-checking and production smoke tests to CI.
File summaries
| File | Description |
|---|---|
.github/workflows/node.js.yml |
Adds checks and smoke tests. |
.gitignore |
Ignores generated themes and test results. |
astro.config.mjs |
Revises CSS handling and chunk splitting. |
azure-pipelines/build-pipeline.yml |
Removes obsolete Azure pipeline. |
package-lock.json |
Locks Playwright dependencies. |
package.json |
Adds theme and test scripts. |
playwright.config.ts |
Configures production smoke tests. |
public/styles/layout.css |
Adds pre-paint navigation and component guards. |
scripts/copy-ig-themes.js |
Copies theme assets into public output. |
src/components/NavSidebar.astro |
Removes server-rendered navigation. |
src/layouts/SampleLayout.astro |
Adds head styles and client navigation shell. |
src/pages/[...slug].astro |
Resolves styles and loads sample modules. |
src/pages/index.astro |
Uses shared sample enumeration. |
src/pages/nav.json.ts |
Exposes the static navigation tree. |
src/scripts/nav.ts |
Implements client-side navigation. |
src/utils/samples.ts |
Centralizes sample and stylesheet resolution. |
tests/smoke/samples.spec.ts |
Adds production regression coverage. |
Review details
- Files reviewed: 15/17 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The index armed the sidebar flag from the deferred nav module, so a click before it ran reached the sample without a sidebar. vnext did this synchronously; restore that in the pre-paint inline script. This also removes the module's unguarded sessionStorage writes. The smoke tests ignored every resource failure, so a missing first-party asset (e.g. a copied theme sheet) passed CI. Ignore only failures from external origins.
ChronosSF
approved these changes
Sep 2, 2026
ChronosSF
added a commit
that referenced
this pull request
Sep 2, 2026
Merge pull request #1253 from IgniteUI/rkaraivanov/app-architecture-i…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The nav sidebar was server-rendered into all 962 pages (~185MB of dist), Rollup hosted shared library code inside arbitrary sample chunks (11MB chunks, wrong-theme leaks), samples flashed unstyled until their JS chunk evaluated, and CI only built.