page updates

[rshanthini]

No description provided.

[rshanthini]

page updates

[Copilot]

🟡 Changes recommended

There are confirmed runtime-breaking UI paths plus security/operational concerns (unvalidated query params and high-risk GitHub API/rate-limit dependency) that should be addressed before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refreshes the Labs web experience by adding a dedicated lab detail page and updating the main catalog page to support theme switching and a redesigned layout for discovering and launching labs.

Changes:

• Added labs/lab.html to render an individual lab’s markdown (and optional deck link) into a full detail page.
• Updated labs/index.html with light/dark theming, refreshed branding/hero sections, and a new needs-driven catalog browsing UI.
• Switched lab launching to open the new detail page (and optional presentation link) in a separate tab.

File summaries

File	Description
labs/lab.html	New lab detail page that fetches markdown, parses frontmatter, and renders formatted HTML with theme support.
labs/index.html	Catalog UI redesign with theme switcher, new filtering/tabs, and dynamic lab discovery via GitHub API + frontmatter parsing.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There are confirmed functional/security issues in the new navigation and content-loading logic (branch/deck URL handling, path validation, and catalog filtering) that can break links or load unintended resources.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

labs/lab.html:616

• deck comes from URLSearchParams.get(), which decodes %20 back to spaces. Setting deckLink.href = deck will produce a URL with literal spaces, which can break deck downloads (your fallback deck filenames include spaces).
  labs/index.html:1075
• renderLabs() no longer renders any elements with data-path, so this listener setup is dead code and can be removed to avoid confusion.

labs/lab.html:610

• The lab path query param is used directly in fetch(path) and in the GitHub repo link. Without validation, a user can provide .. segments or absolute/URL values to fetch unexpected same-origin files or construct unexpected repo URLs. Consider restricting to relative .md paths under the labs site and rejecting traversal/protocols.

      return `<table>${cleanRows.map((row, index) => {
        const cells = row.replace(/^\||\|$/g, "").split("|").map(cell => cell.trim());
        const tag = index === 0 ? "th" : "td";
        return `<tr>${cells.map(cell => `<${tag}>${renderInlineMarkdown(cell, basePath)}</${tag}>`).join("")}</tr>`;
      }).join("")}</table>`;

• Files reviewed: 2/8 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

There are confirmed functional/reliability issues (branch URL encoding, branch not propagated to lab.html, and catalog load aborting on a single fetch failure) that can break navigation or leave the catalog empty.

Review details

Suppressed comments (7)

Previously missed (4) — in code that hasn't changed since the last review.

labs/index.html:772

• The "Coming soon" CTA is marked up as a but it is not keyboard-focusable and doesn’t provide native button semantics, which can confuse assistive technologies. Prefer a real disabled (or remove role="button" if it’s purely decorative).
  labs/index.html:901
• repoUrl() percent-encodes the entire branch name, which breaks common branch names containing slashes (e.g. feature/foo becomes feature%2Ffoo and GitHub won’t resolve the blob URL). Encode branch path segments while preserving /.
  labs/index.html:996
• loadCatalogMetadata() aborts the entire catalog if any single markdown fetch fails because the per-item loader throws inside Promise.all(...). A single missing/renamed lab file will make the whole page show an error and render zero labs. Handle per-item failures and skip invalid entries instead of rejecting the whole load.
  labs/index.html:1000
• normalize() is now unused (no call sites remain), so it’s dead code and can be removed to keep the page script minimal.

This issue also appears on line 1078 of the same file.

labs/index.html:1080

• renderLabs() no longer renders any elements with data-path, but it still tries to attach click handlers to [data-path]. This is now dead code (and openLab() becomes unreachable) since labs are opened via the new links.

      labGrid.querySelectorAll("[data-path]").forEach(button => {
        button.addEventListener("click", () => openLab(button.dataset.path));
      });

labs/index.html:1074

• The lab details link doesn’t carry the current branch query parameter, so when previewing a non-main branch the "Open in repo" link inside lab.html will still point at main (and may not match what was previewed). Pass branch through in the generated lab.html URL.

          <div class="lab-actions">
            <a class="lab-link" href="lab.html?path=${encodeURIComponent(lab.path)}&branch=${encodeURIComponent(branch)}${lab.deck ? `&deck=${encodeURIComponent(lab.deck)}` : ""}" target="_blank" rel="noopener">Lab</a>
            ${lab.deck ? `<a class="deck-link" href="${encodeURI(lab.deck)}" target="_blank" rel="noopener">Presentation</a>` : ""}

labs/lab.html:468

• repoUrl() is hard-coded to the main branch, so when index.html is previewed with a ?branch=... query, the lab detail page’s “Open in repo” link can point at the wrong branch. Also, branch names can contain /, so encode path segments rather than the whole ref.

    function repoUrl(path) {
      const branch = params.get("branch") || "main";
      return `https://github.com/microsoft/apps-agents-workshop/blob/${encodeURIComponent(branch)}/labs/${path.split("/").map(encodeURIComponent).join("/")}`;

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Lite

[rshanthini]

Copilot can you fix the recent comments?

[Copilot]

🟡 Changes recommended

Current catalog and Markdown-rendering defects break navigation, formatting, and metadata display.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (9)

Previously missed (5) — in code that hasn't changed since the last review.

labs/index.html:416

• This sticky filter is inside main, which has overflow: hidden at line 156. That ancestor becomes the sticky containing block, so the tabs will not stick to the viewport while the document scrolls; use an ancestor that does not establish an overflow scroll container (for example, change the clipping strategy to overflow: clip).
  labs/lab.html:530
• Rejecting a second .. segment turns valid repository links into #. Current labs contain many links such as ../../solutions/README.md (and prereqs.md uses ../solutions/README.md), so their required setup links do nothing on the rendered page. Resolve paths that leave labs/ to a branch-aware repository URL rather than discarding them.
  labs/lab.html:581
• The inline renderer handles bold and code but not emphasis, so existing lines such as *Enable Power Apps and custom tools...* and _Power CAT | ..._ display their delimiter characters instead of italic text. Add emphasis parsing that protects code/link tokens, or use a CommonMark renderer.
  labs/lab.html:714
• The list regexes discard indentation and explicit ordered markers. Current labs include nested numbered steps (for example, step 6 with indented steps 1–5 in byoc-powerapps.md), and ordered sequences are also split by code/images; this renderer flattens them and restarts each <ol> at 1, changing the instructional hierarchy and numbering.
  labs/lab.html:698
• Thematic-break lines fall through to paragraph.push, so current documents such as canvas-apps-and-dataverse-mcp.md render each --- separator as literal text instead of an <hr>. Recognize thematic breaks before the generic paragraph path.

labs/index.html:791

• Branch names containing / are still not encoded as one {ref} path parameter. For a ref such as feature/page-updates, this builds /commits/feature/page-updates, so discovery can 404 and silently use the stale fallback catalog instead of the selected branch.
  labs/index.html:468
• --ps-card-bg is never defined in either theme, so this declaration is invalid and empty/error messages render without the intended surface background. Use an existing surface variable.

      grid-column: 1 / -1;

labs/lab.html:625

• The generated slug collapses punctuation and surrounding spaces into one hyphen, but authored fragments follow GitHub-style slugs that preserve two separators around removed punctuation. For example, Part 1 — Enable... is generated as part-1-enable... while the current TOC links to #part-1--enable..., leaving those navigation links broken.

          .replace(/\[([^\]]+)\]\([^)]+\)/g, "$1")
          .replace(/[`*_~]/g, "")

labs/lab.html:573

• CommonMark autolinks are still left as escaped text because this replacement chain never handles <https://...>. Existing instructions use forms such as <https://make.powerapps.com>, so users see literal angle brackets instead of a clickable link; add safe HTTP(S) autolink handling before restoring HTML tokens or use a complete Markdown renderer.

      return escapeHtml(escapedValue)
        .replace(/<(https?:\/\/[^&\s]+)>/gi, (_, url) => {
          const resolved = decodeMarkdownUrl(url);
          const rendered = escapeHtml(resolved);
          return `<a href="${rendered}" target="_blank" rel="noopener" aria-label="${rendered} (opens in a new tab)">${rendered}</a>`;

• Files reviewed: 2/8 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Existing links, list numbering, emphasis, and business-need filtering are rendered incorrectly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

labs/lab.html:529

• When a relative Markdown link climbs above labs/, this returns #, so existing prerequisite links such as ../../solutions/README.md are dead on the rendered page (this pattern appears in multiple catalog labs, including mcp-apps.md:63 and automations-foundation/README.md:22). Normalize valid repository-relative targets against the repository root and emit a selected-branch GitHub URL for files outside labs/, while still rejecting paths that escape the repository.
  labs/lab.html:587
• Single-asterisk emphasis is not rendered, even though current labs use it extensively (for example, automations-foundation/01-cloud-flow.md:502 contains *service auth*, *secrets...*, and *not leaking*). Those pages therefore show literal asterisks instead of emphasized text. Add safe <em> handling or use a Markdown parser that supports standard inline emphasis.

labs/index.html:850

• Assigning an empty tag list makes every metadata-less catalog entry disappear from all business-need filters. This regresses existing mappings for prereqs.md, automations-foundation/README.md, and the IP firewall, IP cookie binding, and CMK module READMEs, which were previously categorized under governance or automation. Preserve those fallback mappings or add equivalent frontmatter before switching these entries to metadata-driven filtering.

          if (!hasRequiredFrontMatter(metadata)) {
            metadata.title = heading;
            metadata.level = "—";
            metadata.duration = "Self-paced";
            metadata.persona = "All learners";
            metadata.tags = [];
            metadata.description = description || "Explore this workshop resource.";

• Files reviewed: 2/8 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Fixed: same-indent ordered and unordered groups now split correctly while nested items stay attached to their parent list. I also validated the renderer with a mixed-list scenario.

[Copilot]

🔵 Needs a closer look

Markdown rendering and branch-aware PDF resolution have unresolved functional defects.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

labs/lab.html:707

• The fence matcher accepts at most four leading spaces, but dataverse-business-skills.md:163-166 nests a fenced block seven spaces under a list. That block falls through to paragraph rendering, exposing the backticks and collapsing the commands instead of rendering code. Accept the indentation used by nested fences.
  labs/lab.html:789
• Thematic-break markers currently fall through into paragraph text. Existing catalog labs use these extensively (for example, canvas-apps-dataverse-mcp.md:21,37,45 and byoc-powerapps.md:940), so the detail page displays literal ---/*** instead of separators. Handle thematic breaks before accumulating paragraph content.

labs/lab.html:546

• Only images are resolved through the selected branch. The current Dataverse lab links to resources/Northwind_Traders_Discount_Policy.pdf, so with ?branch=... that download still comes from the deployed site branch and may be stale or missing. Include PDF assets in the branch-aware resolution.

      const isLocalPreview = ["localhost", "127.0.0.1"].includes(window.location.hostname);
      const isImageAsset = /\.(?:png|jpe?g|gif|svg|webp|avif)$/i.test(trimmed);
      const isSafeRelative = !trimmed.startsWith("../") && !trimmed.includes("..");
      return (!isLocalPreview && isImageAsset && isSafeRelative) ? contentUrl(resolvedRelative) : resolvedRelative;

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[rshanthini]

🔵 Needs a closer look

Markdown rendering and branch-aware PDF resolution have unresolved functional defects.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

labs/lab.html:707

• The fence matcher accepts at most four leading spaces, but dataverse-business-skills.md:163-166 nests a fenced block seven spaces under a list. That block falls through to paragraph rendering, exposing the backticks and collapsing the commands instead of rendering code. Accept the indentation used by nested fences.
  labs/lab.html:789
• Thematic-break markers currently fall through into paragraph text. Existing catalog labs use these extensively (for example, canvas-apps-dataverse-mcp.md:21,37,45 and byoc-powerapps.md:940), so the detail page displays literal ---/*** instead of separators. Handle thematic breaks before accumulating paragraph content.

labs/lab.html:546

• Only images are resolved through the selected branch. The current Dataverse lab links to resources/Northwind_Traders_Discount_Policy.pdf, so with ?branch=... that download still comes from the deployed site branch and may be stale or missing. Include PDF assets in the branch-aware resolution.

      const isLocalPreview = ["localhost", "127.0.0.1"].includes(window.location.hostname);
      const isImageAsset = /\.(?:png|jpe?g|gif|svg|webp|avif)$/i.test(trimmed);
      const isSafeRelative = !trimmed.startsWith("../") && !trimmed.includes("..");
      return (!isLocalPreview && isImageAsset && isSafeRelative) ? contentUrl(resolvedRelative) : resolvedRelative;

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

Copilot fix this

[Copilot]

🔵 Needs a closer look

Markdown rendering and branch-aware PDF resolution have unresolved functional defects.

...

Fixed in commit 5a68463:

• Code fences now match any leading indentation, so nested fences (e.g. the 7-space block in dataverse-business-skills.md) render as code instead of literal backticks.
• Thematic breaks (---, ***, ___) are handled before paragraph accumulation and render as <hr> (verified: 10 separators in canvas-apps-and-dataverse-mcp.md, 8 in byoc-powerapps.md, no literal dashes left).
• PDF assets are included in branch-aware resolution, so resources/Northwind_Traders_Discount_Policy.pdf now resolves through the selected branch.

[Copilot]

🔵 Needs a closer look

Metadata fallbacks break filtering and summaries, while the Markdown renderer mishandles existing document structures.

Review details

Suppressed comments (5)

Previously missed (5) — in code that hasn't changed since the last review.

labs/index.html:842

• The fallback description still selects attribution boilerplate. For example, automations-foundation/README.md has Built by: Power CAT as its first eligible line, so its catalog card shows that instead of the descriptive paragraph immediately below. Exclude Built by: lines when deriving summaries.
  labs/index.html:849
• Fallback entries lose all business-need classification because metadata.tags is forced to []. Several discovered catalog sources without frontmatter—such as automations-foundation/README.md and the IP firewall, IP cookie binding, and CMK READMEs—previously belonged to automation or governance filters, but now disappear whenever users select those needs. Preserve curated fallback tags or add equivalent frontmatter before building the filters.
  labs/lab.html:640
• Joining paragraph lines with a plain space discards Markdown hard breaks because the lines are also trimmed before flushing. Many current labs intentionally end image lines with two spaces before a Figure: caption (for example managed-security/ip-firewall/README.md:43-44), so the caption is emitted inline with the image instead of on the next line. Preserve trailing-space breaks and render them as <br>.
  labs/lab.html:693
• Flattening every blockquote line into one paragraph removes nested list semantics. The existing note in in-app-agents/in-app-agents.md:185-189 contains three quoted bullets, but this renders them as literal hyphen-prefixed text, so assistive technology no longer exposes a list. Parse blockquote contents as block Markdown rather than joining all lines with <br>.
  labs/lab.html:838
• The detail-page fallback has the same attribution problem: opening a no-frontmatter module such as automations-foundation/README.md sets the hero summary to Built by: Power CAT rather than the module description. Skip attribution lines when choosing the fallback summary.

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[rshanthini]

🔵 Needs a closer look

Metadata fallbacks break filtering and summaries, while the Markdown renderer mishandles existing document structures.

Review details

Suppressed comments (5)

Previously missed (5) — in code that hasn't changed since the last review.

labs/index.html:842

• The fallback description still selects attribution boilerplate. For example, automations-foundation/README.md has Built by: Power CAT as its first eligible line, so its catalog card shows that instead of the descriptive paragraph immediately below. Exclude Built by: lines when deriving summaries.
  labs/index.html:849

• Fallback entries lose all business-need classification because metadata.tags is forced to []. Several discovered catalog sources without frontmatter—such as automations-foundation/README.md and the IP firewall, IP cookie binding, and CMK READMEs—previously belonged to automation or governance filters, but now disappear whenever users select those needs. Preserve curated fallback tags or add equivalent frontmatter before building the filters.
  labs/lab.html:640

• Joining paragraph lines with a plain space discards Markdown hard breaks because the lines are also trimmed before flushing. Many current labs intentionally end image lines with two spaces before a Figure: caption (for example managed-security/ip-firewall/README.md:43-44), so the caption is emitted inline with the image instead of on the next line. Preserve trailing-space breaks and render them as <br>.
  labs/lab.html:693

• Flattening every blockquote line into one paragraph removes nested list semantics. The existing note in in-app-agents/in-app-agents.md:185-189 contains three quoted bullets, but this renders them as literal hyphen-prefixed text, so assistive technology no longer exposes a list. Parse blockquote contents as block Markdown rather than joining all lines with <br>.
  labs/lab.html:838

• The detail-page fallback has the same attribution problem: opening a no-frontmatter module such as automations-foundation/README.md sets the hero summary to Built by: Power CAT rather than the module description. Skip attribution lines when choosing the fallback summary.

• Files reviewed: 2/8 changed files

• Comments generated: 0 new

• Review effort level: Balanced

Copilot fix this

[Copilot]

🔵 Needs a closer look

Metadata fallbacks break filtering and summaries, while the Markdown renderer mishandles existing document structures.

...

Fixed in commit 69b9c14:

• Fallback descriptions (catalog card and lab hero) now skip Built by: attribution lines in addition to headings and HTML comments, so automations-foundation/README.md shows its introductory paragraph.
• Fallback catalog entries without frontmatter keep curated business-need tags (automations-foundation, IP firewall, IP cookie binding, and CMK READMEs), so they stay discoverable under the automation and governance filters.
• Paragraph rendering preserves Markdown hard breaks (trailing two spaces) and emits <br>, so Figure: captions render on their own line under the image.
• Blockquote content is parsed as block Markdown, so quoted lists (for example in-app-agents.md:185-189) render as real <ul> elements.

Verified by rendering all 53 lab Markdown files (471 hard breaks and 265 blockquotes rendered, no errors).

[Copilot]

🟢 Approval recommended

The current changes address the previously identified correctness, security, rendering, and branch-handling issues.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Broken dark-mode imagery, Markdown rendering errors, and keyboard focus loss remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

All changed files were reviewed and previously identified functional, security, rendering, and branch-handling issues are resolved.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The large custom Markdown parser and remote branch-discovery flow warrant final human validation across representative lab content and browsers.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[rshanthini]

🔵 Needs a closer look

The large custom Markdown parser and remote branch-discovery flow warrant final human validation across representative lab content and browsers.

Review details

Copilot fix it

[Copilot]

🔵 Needs a closer look

The large custom Markdown parser and remote branch-discovery flow warrant final human validation across representative lab content and browsers.

...

Fixed in commit b8f6f2e: completed representative Markdown validation across all 53 lab files and added the missing CAUTION GitHub alert matcher so supported alerts render consistently. Branch/resource URL and dark-theme asset checks also pass.

[Copilot]

🟢 Approval recommended

The updated implementation resolves the previously identified functional, security, branch-handling, rendering, and accessibility issues.

Review details

• Files reviewed: 2/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced