Skip to content

fix(web): remove vestigial className="app-root" tokens#31389

Merged
ashleeradka merged 1 commit into
mainfrom
lum-1769-remove-vestigial-app-root
May 20, 2026
Merged

fix(web): remove vestigial className="app-root" tokens#31389
ashleeradka merged 1 commit into
mainfrom
lum-1769-remove-vestigial-app-root

Conversation

@ashleeradka
Copy link
Copy Markdown
Contributor

Summary

PR #31324 deleted AppRootProvider after confirming zero CSS rules target .app-root in this repo — theme tokens now live on :root and apply via data-theme on <html>. Two className="app-root" strings survived that cleanup and have been dead ever since (they target no CSS rule). A docstring example in portal-container.tsx also still illustrated the old .app-root wrapper pattern.

This PR removes the dead tokens and updates the docstring to reflect post-#31324 conventions.

Changes

  • apps/web/src/components/native-splash.tsx — drop app-root from className (other classes preserved).
  • apps/web/src/domains/account/pages/login-page.tsx — drop app-root from className (other classes preserved).
  • packages/design-library/src/utils/portal-container.tsx — rewrite docstring example. The provider is no longer needed at the app shell (overlays fall back to document.body, which now has access to design tokens via :root). The example now shows the provider's primary remaining use case: nesting overlays inside a dialog so menus opened from within a modal portal into the modal rather than escaping to document.body.

No runtime behavior changes — only dead className tokens removed and a docstring updated.

Verification

  • grep -rn "app-root" apps/web/src packages/design-library/src returns no matches.
  • cd apps/web && bunx tsc --noEmit passes cleanly.

Links

Closes LUM-1769

https://claude.ai/code/session_01DGgkooyEB8d6v9fpDj7UxN


Generated by Claude Code

PR #31324 deleted AppRootProvider after confirming zero CSS rules
target `.app-root` in this repo — theme tokens live on `:root` and
apply via `data-theme` on `<html>`. Two `className="app-root"` strings
survived that cleanup and are dead (target no CSS rule). The
portal-container docstring example also still referenced the old
wrapper pattern; it now reflects current conventions where overlays
fall back to `document.body` directly and the provider is reserved
for nested overlay scoping (dialogs, shadow DOM).

Closes LUM-1769
@linear
Copy link
Copy Markdown

linear Bot commented May 20, 2026

LUM-1769

@ashleeradka ashleeradka marked this pull request as ready for review May 20, 2026 23:43
Copy link
Copy Markdown
Contributor

@vex-assistant-bot vex-assistant-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

Dead code sweep — exactly right. Three surgical removals, no behavioral changes.

  • native-splash.tsx + login-page.tsxapp-root was a no-op className targeting no CSS rule since #31324 deleted AppRootProvider and moved theme tokens to :root / data-theme on <html>. Both removals preserve all other classes cleanly.

  • portal-container.tsx docstring — the rewrite is better than the original: correctly reflects that document.body is now a valid default fallback (tokens on :root), demotes the app-shell mount pattern from "primary use" to "not needed," and updates the example to show the actual remaining use case (dialog nesting with role="dialog"). Accurate, concise, actionable.

Verification checks in the PR description (grep -rn "app-root" returns empty, TypeScript clean) are exactly what this kind of change warrants. CI fully green.

Trigger @codex review + @devin review this PR for second approval gate.

@vex-assistant-bot
Copy link
Copy Markdown
Contributor

@codex review

@vex-assistant-bot
Copy link
Copy Markdown
Contributor

@devin review this PR

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ashleeradka ashleeradka merged commit a6fd005 into main May 20, 2026
20 checks passed
@ashleeradka ashleeradka deleted the lum-1769-remove-vestigial-app-root branch May 20, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants