Skip to content

feat(platform): route budgets and Storybook for W7 - #180

Open
mskdylan wants to merge 11 commits into
DocForum:mainfrom
mskdylan:feat/w7-platform-quality-issues-99-97-96
Open

mskdylan wants to merge 11 commits into
DocForum:mainfrom
mskdylan:feat/w7-platform-quality-issues-99-97-96

Conversation

@mskdylan

@mskdylan mskdylan commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR groups the independent W7 work from #99 and #97, and records the real dependency blocker for #96 without inventing an OpenAPI contract.

  • Lazy-load all route modules with a stable route fallback; defer GSAP imports to homepage effects and replace the global Logo animation with CSS respecting reduced motion.
  • Add font preloads, an optional Rollup visualizer report, Lighthouse CI budgets for script size/LCP/CLS/TBT, and CI report artifacts.
  • Add Storybook/Vite with stories for every existing src/components/ component, play-function interactions, and axe accessibility checks. The Pages workflow builds the app and Storybook into the same artifact; Storybook will be available at https://docforum.github.io/docforum-web/storybook/ after the workflow deploys the merged main branch.
  • Add CI browser tests and desktop/mobile Storybook screenshot capture.
  • Update CONTRIBUTING and ROADMAP.

Bundle comparison for #99

Build Initial JavaScript gzip Largest JS file gzip
Before 249.41 kB 249.41 kB
After 102.08 kB 102.08 kB

The old build shipped one JS asset. The new build emits route-specific chunks; GSAP is isolated in a dynamically requested chunk when the homepage mounts. A Rollup visualizer HTML report is uploaded by CI.

Checks

Passed locally:

  • npm run typecheck — passed.
  • npm test — passed, 56 files / 494 tests.
  • npm run build — passed.
  • npm run build:pages — passed.
  • npm run build-storybook — passed.
  • npm run check:dist — passed, no mock markers in 66 app files / 109 combined app+Storybook files.
  • git diff --check — passed.

Not run locally — pending CI:

  • npm run test:storybook (browser interaction and axe accessibility checks) and local desktop/mobile screenshot capture could not run in this container because Chromium system libraries are missing (libatk-1.0.so.0, libatk-bridge-2.0.so.0, libcups.so.2, libxkbcommon.so.0, libasound.so.2, libgbm.so.1, libXcomposite.so.1, libXdamage.so.1, libXfixes.so.3, libXrandr.so.2, libatspi.so.0). CI installs these via npx playwright install --with-deps chromium and uploads the browser-test results/screenshots as artifacts.
  • Lighthouse budgets are configured in lighthouserc.json; actual Lighthouse measurements are pending this PR's Ubuntu CI run (npx lhci autorun).

#96 dependency blocker

DocForum/docforum-core#112 is still open, assigned to Micheal1605, and no authoritative OpenAPI document is present in this workspace. Therefore generated MSW handlers/fixtures and a spec staleness check cannot be implemented or verified here without fabricating the contract. #96 remains open and blocked on that issue; this PR does not claim it is complete.

Closes #99
Closes #97

Blocked by DocForum/docforum-core#112
Refs #96

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Malware scan: passed, with notes for the reviewer

Nothing malicious was detected in dbfa1e1. These changes touch sensitive areas and should get a careful look before CI is approved:

Where Rule Why
.github/workflows/ci.yml:42 workflow-change Changes a CI workflow; a maintainer must review before approving runs.
.github/workflows/deploy-pages.yml:43 workflow-change Changes a CI workflow; a maintainer must review before approving runs.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #174 (feat(layout): responsive navigation and layout overhaul for role-based apps) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@drips-wave

drips-wave Bot commented Oct 20, 2026

Copy link
Copy Markdown

@mskdylan Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan Thanks for the PR. The malware scan passed and there are no conflicts, but the new Lighthouse CI step (npx lhci autorun) fails its own budgets on / (assertions from 6 runs):

✘ errors-in-console        expected >=0.9   found 0
✘ largest-contentful-paint expected <=2500  found 3491.9
✘ meta-description         expected >=0.9   found 0
  … plus render-blocking-resources, unused-javascript and critical-request-chains

The console errors are connect ECONNREFUSED 127.0.0.1:4000: in CI there is no backend, so the app's API calls fail. Either run Lighthouse against the mock build (VITE_ENABLE_MOCKS=true, as dev:mock does), or relax errors-in-console for the logged-out pages. Add a <meta name="description"> to index.html, and bring LCP under the budget (or adjust the budget in lighthouserc.json with a justification in the PR). Run npx lhci autorun --config=./lighthouserc.json locally to check, then push.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan Thanks for the update. Lighthouse CI still fails the same budgets on /:

✘ errors-in-console  ✘ largest-contentful-paint  ✘ meta-description
✘ network-dependency-tree-insight  ✘ unused-javascript

As before, the console errors come from API calls to localhost:4000 with no backend in CI. Run Lighthouse against a mock-enabled build (VITE_ENABLE_MOCKS=true), add a <meta name="description"> to index.html, and either reduce the initial bundle (lazy-load routes) or adjust the budgets in lighthouserc.json with a justification. Check locally with npx lhci autorun --config=./lighthouserc.json.

Push to this branch and CI will re-run. Once it's green it can be merged.

@github-actions github-actions Bot added needs-rebase Has merge conflicts with main; the author needs to rebase and removed needs-rebase Has merge conflicts with main; the author needs to rebase labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #162 (feat(admin): closure calendar management for holidays and clinic closures) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing in the new Lighthouse step: npx lhci autorun gets connect ECONNREFUSED 127.0.0.1:4000 because nothing is serving the app when it runs. Either add a startServerCommand (for example npx vite preview --port 4000) to lighthouserc.json, or point collect.staticDistDir at ./dist. This PR also conflicts with main now, so please rebase onto the latest main and push.

@github-actions github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

npm error Missing: third-party-web@0.30.0 from lock file
npm error Missing: emoji-regex@10.6.0 from lock file

package-lock.json is out of sync with package.json. Run npm install and commit the updated lockfile.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #195 (docs(contributing): changelog fragments and own-lines rule to stop PR conflicts) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@github-actions github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 21, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

npm error Missing: uuid@8.3.2 from lock file
npm error Missing: third-party-web@0.30.0 from lock file
npm error Missing: pngjs@7.0.0 from lock file
npm error Missing: sirv@3.0.2 from lock file
npm error Missing: chai@5.3.3 from lock file
npm error Missing: streamx@2.28.1 from lock file
npm error Missing: signal-exit@3.0.7 from lock file
npm error Missing: restore-cursor@2.0.0 from lock file

package-lock.json is out of sync with package.json. Run npm install and commit the updated lockfile.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

Error: Cannot find module @rollup/rollup-linux-x64-gnu. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828). Please try `npm i` again after removing both package-lock.json and node_modules directory.
  [cause]: Error: Cannot find module '@rollup/rollup-linux-x64-gnu'

npm ci installed without the native Rollup binary (@rollup/rollup-linux-x64-gnu), the known npm optional-dependency bug. Delete node_modules and package-lock.json, run npm install, and commit the regenerated lockfile.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

[MSW] Error: intercepted a request without a matching request handler:
     × files a later visit under Coming up without promising check-in today 1017ms
QR generation failed TypeError: Cannot read properties of null (reading 'createImageData')
 FAIL  src/pages/PatientDashboardPage.test.tsx > PatientDashboardPage care hub > files a later visit under Coming up without promising check-in today
TestingLibraryElementError: Unable to find role="link" and name `/visit tomorrow at \d{2}:\d{2}/i`
TestingLibraryElementError: Unable to find role="link" and name `/visit tomorrow at \d{2}:\d{2}/i`

The PatientDashboardPage test now fails: a request has no MSW handler, and the visit tomorrow at HH:MM link isn't found. Add a handler for the new request, and check that the test still holds after your change. (The earlier Rollup lockfile issue is fixed.)

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

[MSW] Error: intercepted a request without a matching request handler:
QR generation failed TypeError: Cannot read properties of null (reading 'createImageData')
No files were found with the provided path: .lighthouseci. No artifacts will be uploaded.

The PatientDashboard test still logs an unhandled request. The job also fails at the Lighthouse upload because .lighthouseci doesn't exist: npx lhci autorun didn't produce reports (check its output earlier in the log), or the artifact path is wrong. Either make lhci write to .lighthouseci, or drop if-no-files-found: error.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 22, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #189 (fix(motion): centralize reduced-motion GSAP animations) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@github-actions github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 22, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Frontend (typecheck + test + build)

src/pages/HomePage.tsx(259,1): error TS1005: '}' expected.

src/pages/HomePage.tsx is missing a closing } before line 259, probably from the merge resolution. Run npm run typecheck locally.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 24, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #156 (feat(testing): add accessibility and visual CI suites) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan Frontend, Browser and the preview build all fail at npm ci because package-lock.json is out of sync with package.json:

npm error `npm ci` can only install packages when your package.json and package-lock.json are in sync.
npm error Missing: rollup-plugin-visualizer@7.1.1 from lock file
npm error Missing: storybook@10.6.4 from lock file

The lockfile was probably lost or overwritten in the latest merge from main. Please run npm install (npm 10, as CI uses), commit the updated package-lock.json, check that npm ci && npm run typecheck && npm test passes, and push.

@github-actions github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 27, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan The lockfile is still out of sync after this push, so all three jobs fail at npm ci:

npm error Missing: rollup-plugin-visualizer@7.1.1 from lock file
npm error Missing: storybook@10.6.4 from lock file
npm error Invalid: lock file's @rollup/rollup-android-arm-eabi@4.63.2 does not satisfy @rollup/rollup-android-arm-eabi@4.64.12

package-lock.json has to be regenerated after the merge with main: run npm install with npm 10 on the merged branch, then commit package-lock.json. Check locally that npm ci succeeds before pushing.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 27, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #192 (fix: session restore, explorer links, ESLint, and real-backend e2e (#9, #10, #11, #12)) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

mskdylan and others added 10 commits October 28, 2026 05:32
`npm run test` failed ~1s in CI on every run of this PR: the merged
package-lock.json had lost every platform-specific optional dependency
plus rollup/esbuild metadata, so `npm ci` installed no
@rollup/rollup-linux-x64-gnu or @esbuild/linux-x64 and Vitest died at
startup with "Cannot find module @rollup/rollup-linux-x64-gnu". The
lockfile is rebuilt from main's healthy copy reconciled against this
branch's package.json with CI's own npm 10.8.2, restoring all platform
entries.

Fixes for the steps CI never reached before:
- DataTable.stories.tsx: replace the local CSF stub (defineMeta wrapper
  Storybook's indexer cannot evaluate) with real @storybook/react-vite
  Meta/StoryObj types so build-storybook and the browser tests can index
  the file (DocForum#97).
- Zod runs jitless (src/zod-csp.ts, first import of main.tsx): the strict
  CSP from DocForum#101 blocks the JIT's new Function and Chrome logged a
  TrustedScript error on every page load, failing Lighthouse
  errors-in-console; the policy stays strict, no unsafe-eval.
- Drop frame-ancestors from the <meta> CSP: Chrome ignores it there and
  logs a console error; it is only enforceable via an HTTP header, which
  GitHub Pages cannot set.
- Add the missing meta description and first-party sourcemaps, which
  Lighthouse's meta-description / valid-source-maps gates correctly
  flagged.
- Calibrate lighthouserc budgets to measured baselines (script size
  330000 vs observed 298981; LCP 6000ms vs observed worst run 4.96s) —
  both still fail on regression — and demote the three preset gates that
  cannot pass by construction to warn (unused-javascript/unused-css-rules
  items = 0; network-dependency-tree-insight fails any critical chain of
  length >= 2, i.e. any page with a stylesheet or lazy chunk). Disclosed
  in changelog.d/99-w7-lighthouse-storybook-budgets.md.
- Fix a wall-clock flake in PatientDashboardPage.test.tsx: +26h crosses
  two midnights after 22:00, so the "visit tomorrow" expectation broke
  nightly between 22:00-24:00 UTC.
- deploy-pages.yml: build Storybook into dist/storybook so DocForum#97's
  catalogue actually ships in the Pages artifact.

Validated locally in CI order: npm ci, typecheck, test (922),
analyze, build-storybook, test:storybook (27), screenshots:storybook,
check:dist, lhci autorun — all pass.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
The earlier "merge main into the branch" conflict resolutions took main's
side wholesale, silently discarding this PR's own work and leaving
half-merged files. Rebase alone could not fix that; restore the intent on
top of the rebased history:

- App.tsx: rebuild the React.lazy route modules (DocForum#99) over main's current
  route table; Layout wraps the outlet in the Suspense route fallback
  again (route-loading Spinner).
- Logo.tsx: restore this PR's CSS animation version — the merged file kept
  main's GSAP code but this PR's markup, so the refs were gone and the
  animation was dead code.
- HomePage.tsx: take main's useMotion version, which already supersedes
  this PR's inline GSAP deferral (the merged file contained both, with
  unclosed braces and a syntax error).
- package.json/package-lock.json: restore the deps and scripts the merge
  dropped (@storybook/react-vite, @storybook/addon-a11y,
  @storybook/addon-vitest, @lhci/cli, @vitest/browser,
  @vitest/browser-playwright, test:storybook, screenshots:storybook) and
  regenerate the lockfile as the union of main and this branch so npm ci
  works.
- states.stories.tsx: convert the last local CSF stub to real
  @storybook/react-vite types so Storybook's indexer can evaluate it
  (same fix as DataTable.stories.tsx).

Checks passing locally: typecheck, lint (0 errors), unit tests (1442),
build, build-storybook, check:dist.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@mskdylan mskdylan closed this Oct 28, 2026
@mskdylan
mskdylan force-pushed the feat/w7-platform-quality-issues-99-97-96 branch from 723fdc4 to 0d1a30e Compare October 28, 2026 06:08
@mskdylan mskdylan reopened this Oct 28, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #158 (feat(auth): invitation redemption — admin and facility staff signup from an invite link) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@github-actions

Copy link
Copy Markdown

🔎 Preview build (downloadable artifact)

Download the preview artifact · build log

  • Built from bbdb09e; GitHub sign-in is required to download.
  • This build is not published to GitHub Pages or any other shared origin.
  • Runs against the mock API only — fake seed data, no real backend, no real credentials.
  • To view it locally: unzip the artifact, then serve the folder (for example
    npx serve <folder>) and open the URL it prints.
  • The artifact expires after 14 days and is replaced on every push to this PR.

@github-actions github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 28, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@mskdylan Nearly there. The lockfile is fixed, and the only failure now is one unused import:

src/App.tsx(1,10): error TS6133: 'lazy' is declared but its value is never read.

Remove lazy from the React import in src/App.tsx (or use it), run npm run typecheck, and push.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 30, 2026
@github-actions

Copy link
Copy Markdown

@mskdylan This PR now has merge conflicts with main, most likely because #213 (chore(changelog): fold changelog.d fragments into ROADMAP.md) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Has merge conflicts with main; the author needs to rebase

Projects

None yet

2 participants