Skip to content

fix(core): resolve Toast fallback viewport's theme mode instead of OS preference#3743

Open
let-sunny wants to merge 1 commit into
facebook:mainfrom
let-sunny:toast-fallback-theme-mode
Open

fix(core): resolve Toast fallback viewport's theme mode instead of OS preference#3743
let-sunny wants to merge 1 commit into
facebook:mainfrom
let-sunny:toast-fallback-theme-mode

Conversation

@let-sunny

@let-sunny let-sunny commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

useToast()'s fallback viewport (the no-LayerProvider path) can show a fully invisible toast: its body text and dismiss icon compute to the exact same color as the toast's own background. This PR fixes that by re-providing the app's theme mode into the fallback tree, reusing the data-theme attribute that #1587 already keeps in sync on <html>.

The bug fires whenever the app's <Theme mode="light"|"dark"> disagrees with the OS's prefers-color-scheme. And the no-provider setup is not an edge case — it's exactly what useToast.doc.mjs documents and the NoProvider Storybook story demonstrates.

Root cause: the fallback mounts <ToastViewport> via createRoot() on a <div> appended to document.body — a new, disconnected React tree, not a portal. React context doesn't cross that boundary. So when Toast.tsx calls const {mode} = useTheme() to pick its inverted-surface polarity, there's no ThemeContext to read and the hook falls back to OS preference.

#1586/#1587 already solved the CSS half of this disconnected-tree problem by syncing data-astryx-theme/data-theme onto <html>; the JS half — useTheme()'s context-based mode — was never covered. That split is what makes the worst case so bad: the toast's surface color (CSS, fixed) and its text color (JS, still wrong) come from two sources that can disagree, and they converge on the same color.

The fix reads that same data-theme attribute: present means an explicit app mode, absent means mode="system", where the existing OS-preference fallback is already correct, so the provider deliberately supplies nothing.

Aside on why this went unnoticed: the NoProvider story exists precisely to demonstrate the fallback, but the global withTheme decorator in apps/storybook/.storybook/preview.tsx wraps every story in <LayerProvider>. The story that showcases the fallback never actually exercises it.

Change

  • packages/core/src/Toast/useToast.tsx:
    • readRootThemeMode() reads document.documentElement.getAttribute('data-theme') and returns 'light' | 'dark' | null. null covers both mode="system" and no root <Theme> at all — the same "let OS preference decide" semantics useTheme() already has.
    • FallbackThemeProvider wraps the fallback's <ToastViewport> and re-provides ThemeContext with that mode. A MutationObserver on data-theme keeps it live, since fix(theme): sync data-xds-theme to <html> for root provider; add LayerProvider to sandbox #1587 updates the attribute whenever <Theme mode> changes. (Same observer pattern as Outline/useOutlineFromDOM.ts.)
    • The Provider is always rendered; only its value toggles between the derived mode and null. An earlier version omitted the Provider element when mode was unknown — that changes the tree shape on every mode transition, remounts ToastViewport, and silently drops any toast on screen. The new tests caught it.
    • The provided value is {mode, theme: null}. The null is not a shortcut: only the mode is reflected to the DOM, while the theme object exists solely in React context (CSS receives the theme separately, through the theme stylesheet and data-astryx-theme), so there is nothing theme-shaped for the fallback to read. It's also safe for anything that renders inside the fallback tree — toast bodies are arbitrary ReactNode — because useTheme already coalesces ctx?.theme ?? null and resolves a null theme to the default tokens, exactly what the no-provider path returns today. The one field Toast's polarity decision actually reads, mode, is now correct.
  • packages/core/src/theme/useTheme.ts: one type-only change. ThemeContextValue.theme widens to DefinedTheme | null so the provider can express "mode known, theme not" honestly instead of casting. The null-theme state has always existed at runtime via the no-provider path; this just lets the type say so. Theme always provides a real theme, useTheme is the type's only reader (verified repo-wide), and the hook's public return type (UseThemeReturn) is unchanged — no consumer of useTheme() sees a null or gains a null check.
  • packages/core/src/Toast/useToast.test.tsx (new): co-located tests for the fallback's theme provision.
  • .changeset/toast-fallback-theme-mode.md: patch bump for @astryxdesign/core.

Alternatives considered

This PR reads the DOM attribute; the alternative was capturing the real ThemeContext value at the useToast() call site (it runs in the app tree) and re-providing it into the fallback root — the same cross-root plumbing FallbackCapture already does for ToastContext, in the opposite direction, and it would carry the full theme object rather than just the mode. Two reasons for the attribute:

  1. It's the established mechanism for exactly this problem — Toast fallback viewport doesn't receive on-media theme CSS #1586 framed the candidate fixes in attribute terms, and fix(theme): sync data-xds-theme to <html> for root provider; add LayerProvider to sandbox #1587 chose the <html> sync as the robust core fix. This PR consumes the attribute that fix already maintains, adding no new state.
  2. Capturing context inside useToast would subscribe every component that calls the hook to theme changes — a re-render per mode switch across the app, for a hook that is deliberately subscription-free today.

Happy to reshape toward context capture if that's the preferred direction.

Verification

  • New tests (3): the bug case asserts the fallback resolves the app's mode, and fails with the fix reverted; mode="system" keeps today's OS-preference behavior; the LayerProvider path is untouched. Toast/ + theme/ suites: 245/245. Typecheck and lint clean. (The monorepo-wide test run also shows 6 pre-existing packages/cli failures that reproduce identically on a clean upstream/main checkout — A/B verified, unrelated.)

  • End-to-end against the built dist/ output, driven with Playwright (colorScheme: 'dark' emulating OS dark). data-astryx-media is the attribute MediaTheme sets from useTheme()'s resolved mode:

    Scenario (OS pref = dark) data-astryx-media before fix after fix dismiss-icon color before → after
    Fallback, app mode="light" (the bug) light (wrong) dark (correct) rgb(37, 37, 42)rgb(255, 255, 255)
    Fallback, app mode="system" (no regression) light light (unchanged) rgb(37, 37, 42) (unchanged)
    With LayerProvider (control) dark dark (unchanged) rgb(255, 255, 255) (unchanged)

    * (If you re-verify from a source rebuild and the toast background doesn't flip in the fallback case, that's the pre-existing [Bug] Built theme.css pins color-scheme to "light dark", so <Theme mode> can't force light/dark on <html> #3658, fix pending in fix(cli): make theme-build color-scheme decl mode-aware #3660 — unrelated here, and it doesn't affect the data-astryx-media/icon evidence above.)

Repro

import {Theme} from '@astryxdesign/core/theme';
import {useToast} from '@astryxdesign/core/Toast';
import {stoneTheme} from '@astryxdesign/theme-stone/built';
import '@astryxdesign/core/reset.css';
import '@astryxdesign/core/astryx.css';
import '@astryxdesign/theme-stone/theme.css';

function ToastTrigger() {
  const toast = useToast();
  useEffect(() => { toast({body: 'hello'}); }, [toast]);
  return null;
}

// No <LayerProvider> / <AppShell> ancestor — the fallback path.
<Theme theme={stoneTheme} mode="light">
  <ToastTrigger />
</Theme>

With OS prefers-color-scheme: dark emulated (Playwright colorScheme: 'dark'), the dismiss icon and body text compute to the same color as the toast's own background.

Screenshots

Before — the toast in the bottom-right is a blank dark rectangle. Its surface color comes from the CSS side (correct since #1587) while its text/icon polarity comes from the JS mode (wrong in the fallback), and the two converge on the same color:

Fallback toast with invisible content

Control — identical conditions but with LayerProvider. This is also what the fallback renders like after this fix:

Control toast rendering legibly

@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

@let-sunny is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 9, 2026
… preference

useToast()'s fallback viewport mounts via createRoot() on a detached div, so
Toast's useTheme() call can't see ThemeContext and falls back to
prefers-color-scheme. When that disagrees with the app's actual <Theme
mode>, the toast's inverted-surface text/icon can compute to the exact same
color as its own background.

Bridge the gap the same way facebook#1587 already does for CSS: read the mode off
<html data-theme> (present means an explicit app mode, absent means
"system", where the existing OS-preference fallback is already correct) and
re-provide it into the fallback tree via ThemeContext, kept live with a
MutationObserver since that attribute changes whenever <Theme mode> does.
@let-sunny let-sunny force-pushed the toast-fallback-theme-mode branch from d5b3695 to 04d8608 Compare July 9, 2026 22:39
@let-sunny let-sunny marked this pull request as ready for review July 9, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant