fix: address empty space SR announcement in react comps (JAWS & NVDA)

[nk1408]

Pull Request

Description

• Create a new React example app (examples/react-app/) with bar, line, and smooth chart demos using the component, replacing the stale pre-built React example that contained \u2063 (INVISIBLE SEPARATOR) characters causing JAWS to announce "invisible separator"
• Bundle the React example into a single self-contained HTML file (examples/react/index.html) that works when opened via file:// protocol, using vite-plugin-singlefile, IIFE output format, and a custom stripModuleType post-build plugin
• Integrate the React example build into the docs pipeline (scripts/build-site.js) so it always reflects the latest MAIDR source code
• Exclude examples/ and _site/ from tsconfig.json and typedoc.json to prevent typedoc from scanning example source files

examples/react-app/index.html — Vite HTML entry point
examples/react-app/main.tsx — React app with 3 chart examples (bar, line, smooth) using component
examples/react-app/vite.config.ts — Vite config: singlefile plugin, IIFE format, stripModuleType post-build plugin, source aliases

Checklist

• I have read the Contributor Guidelines.
• I have performed a self-review of my own code and ensured it follows the project's coding standards.
• I have tested the changes locally following ManualTestingProcess.md, and all tests related to this pull request pass.
• I have commented my code, particularly in hard-to-understand areas.
• I have updated the documentation, if applicable.
• I have added appropriate unit tests, if applicable.

Additional Notes

[claude]

PR Review: feat: fix empty space SR announcement in react comps (JAWS & NVDA)

Overall: This is a well-motivated fix — eliminating the stale pre-built React example that contained invisible separator characters (U+2063) causing JAWS/NVDA to announce "invisible separator" is the right call. The approach of rebuilding from source is sound. A few issues need attention before merging.

---

1. Gitignore Contradiction (Bug)

# .gitignore
+examples/react/

Adding examples/react/ to .gitignore does not untrack already-committed files. Since examples/react/index.html is already tracked by git (this PR itself updates it with +2137/-182), it will continue to be tracked after this PR merges — the .gitignore entry has no effect on it.

This leaves the repo in a confusing state: the file appears in .gitignore but is still committed and tracked. Future contributors will be confused when git status shows changes to a file that appears in .gitignore.

Choose one approach:

• Option A (track the built file): Remove examples/react/ from .gitignore. Commit it explicitly as a checked-in build artifact (document this in a comment).
• Option B (don't track it): Add git rm --cached examples/react/index.html to the PR, keep the .gitignore entry, and ensure scripts/build-site.js generates it at CI/deploy time.

---

2. Fragile stripModuleType Plugin

// examples/react-app/vite.config.ts
html = html.replace(/<script type="module" crossorigin>/g, '<script>');

This is a brittle post-build string replacement:

• It only matches the exact string <script type="module" crossorigin>. If Vite or vite-plugin-singlefile changes their output format (e.g. adds attributes, changes attribute order), this silently does nothing and the file:// breakage returns.
• Consider using a more robust HTML parser or checking whether the vite-plugin-singlefile package itself has an option for this (it has removeViteModuleLoader and related options).

---

3. require() Introduced Mid-File

// scripts/build-site.js
+const { execSync } = require('node:child_process');
+execSync('npm run build:react-example', { stdio: 'inherit', cwd: ROOT });

The require() call is added inline at the point of use, while all other require() calls in the file are presumably at the top. Move this import to the top of the file alongside the existing requires for consistency.

---

4. process.env: {} is Too Broad

// examples/react-app/vite.config.ts
define: {
  'process.env': {},
},

Replacing the entire process.env object with an empty object will silently swallow any process.env.NODE_ENV checks in the dependency tree, potentially causing libraries to behave incorrectly (e.g., skipping production-mode optimizations). Prefer:

define: {
  'process.env.NODE_ENV': JSON.stringify('production'),
},

---

5. main.tsx is 3,335 Lines of Inlined SVG Data

The entire file is one giant module with three hardcoded SVG blobs (~1,000+ lines each) and their corresponding MaidrData configs. This makes the file:

• Very hard to review in PRs
• Difficult to maintain (any chart update requires editing a giant file)
• Not representative of how users would actually structure their apps

Consider splitting into data/barData.ts, data/lineData.ts, data/smoothData.ts, and keeping main.tsx as a thin entry point. This also makes the example more educational.

---

6. assetsInlineLimit: Infinity Note

build: {
  assetsInlineLimit: Infinity,
  ...
}

This is intentional for the single-file approach, but the resulting HTML file is 2,000+ lines of inlined data. Consider adding a comment explaining why this is set to Infinity so future maintainers don't remove it thinking it's a mistake.

---

Minor: tsconfig.json Exclusion Trade-off

-"exclude": ["node_modules", "dist", "test-react-app"]
+"exclude": ["node_modules", "dist", "test-react-app", "examples", "_site"]

Excluding examples/ from TypeScript checking means type errors in examples/react-app/main.tsx won't surface during npm run type-check. This is acceptable for a generated example, but if the example is hand-authored, consider a separate tsconfig.json inside examples/react-app/ that extends the root config — this way the example is type-checked separately.

---

Summary

Issue	Severity
Gitignore contradiction (tracked file ignored)	High — confusing/broken git state
Fragile stripModuleType regex	Medium — silent failure risk
require() mid-file in build script	Low — style
process.env: {}	Medium — may suppress production optimizations
3335-line monolithic main.tsx	Low — maintainability
Missing comment on assetsInlineLimit: Infinity	Low — maintainability

The gitignore contradiction and the process.env issue are the two most important things to address before merging.

[claude]

PR Review: feat: fix empty space SR announcement in react comps (JAWS & NVDA)

Summary

This PR replaces the stale pre-built React example (which contained invisible separator characters causing JAWS to announce "invisible separator") with a proper build pipeline that generates a self-contained HTML file. The approach is sound and solves the accessibility regression cleanly.

---

Positive Aspects

• Root cause well-identified: Removing the stale, pre-committed bundle and replacing it with a reproducible build pipeline is the right long-term fix.
• stripModuleType plugin: Clever use of a post-build Rollup plugin to strip type="module" and crossorigin so the output works via file:// protocol. The regex approach is more robust than simple string replacement.
• Gitignoring generated output: Moving examples/react/ to .gitignore is correct — generated artifacts should not be tracked.
• tsconfig/typedoc exclusions: Excluding examples/ and _site/ from type-checking and API doc generation is appropriate.
• Build pipeline integration: Tying build:react-example into scripts/build-site.js ensures the example always reflects the latest MAIDR source.

---

Issues to Address

1. Missing newline at end of main.tsx

The diff shows \ No newline at end of file on the last line of examples/react-app/main.tsx. This may fail an eol-last ESLint rule.

2. Missing viewport meta tag in index.html

examples/react-app/index.html lacks <meta name="viewport" content="width=device-width, initial-scale=1.0" />. Without it the page may render incorrectly on mobile/small-screen devices.

3. Accessibility: missing semantic landmark in App()

For a project whose core mission is accessibility, the example app should model best practices. The current root is a <div>, which means screen reader users get no main landmark. Replacing the outer <div> with <main> would fix this with a one-character change — especially important in an accessibility-focused demo.

4. Inline JS function body string — add a comment

In examples/react-app/data/barData.ts the format.x.function field contains a JS function body as a string. This is presumably a MAIDR feature for custom formatting, but it resembles an eval-able string and will raise eyebrows in code review. A brief comment explaining the intent would help future contributors.

5. execSync in build-site.js — improve error messaging

execSync throws on failure (aborting the build — correct behavior), but Node's error output is generic. Consider wrapping with a descriptive try/catch and console.error message to make CI failures easier to diagnose.

6. dangerouslySetInnerHTML — add a safety comment

The SVG components use dangerouslySetInnerHTML. This is safe because the content is statically defined in source files (not user input), but the prop name is intentionally alarming. A short inline comment clarifying it is static, not user-controlled, would reduce concern for future reviewers.

---

Minor Nits

• examples/react-app/index.html should have lang="en" on the <html> tag (WCAG 3.1.1).
• examples/react-app/tsconfig.json duplicates path aliases already in vite.config.ts. This is expected (tsconfig for type-checking, Vite for bundling), but a comment noting the duplication is intentional would help.

---

Overall Assessment

The core change is well-reasoned and correctly solves the JAWS/NVDA announcement bug by eliminating invisible separator characters at their source. The stripModuleType Vite plugin is a creative and maintainable solution for file:// compatibility. The main items worth addressing before merge are #3 (missing <main> landmark) and #2 (missing viewport meta) — both matter for a project whose primary focus is accessibility. Item #1 (missing trailing newline) will likely fail lint in CI.

[xabilitylab]

🎉 This PR is included in version 3.53.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀