rusackas opened a new pull request, #42602:
URL: https://github.com/apache/superset/pull/42602
### SUMMARY
Fixes #40459.
Turned out to be a scoped, traceable root cause, not a wormhole -- fixing
forward rather than needing to disable dark mode.
Every component page under
`developer-docs/components/{ui,design-system,extension}` (the
"Core/Layout/Extension Components" sections from the issue) embeds
live-rendered demos via `docs/src/components/StorybookWrapper.jsx`'s
`StoryExample`/`StoryWithControls`/`ComponentGallery`. Those wrap in
`themeObject.SupersetThemeProvider`, where `themeObject` is a module-level
singleton (`superset-core/src/theme/index.tsx`: `Theme.fromConfig()`)
constructed once with no dark/light config -- it had no way to know about
Docusaurus's own theme toggle, so every live demo always rendered whatever that
default algorithm produced, regardless of the site-wide toggle. The wrapper
boxes around each demo also hardcoded light-only colors (`border: #e8e8e8`,
muted text `#999`/`#666`) as inline styles, so even a correctly-dark-themed
component would've sat inside a stubbornly light box.
**Fix:** mirror Docusaurus's color mode onto the singleton via
`Theme#toggleDarkMode()` -- already exposed by the `Theme` class for exactly
this purpose, just never wired up here -- through a small effect reacting to
`useColorMode()` (`@docusaurus/theme-common`, the same hook the toggle switch
itself uses). Replaced the hardcoded wrapper colors with Docusaurus's own
Infima CSS custom properties (`--ifm-color-emphasis-*`), the same variables the
rest of the site already uses for its (working) dark mode.
**Not addressed here** (flagging as a distinct follow-up, not bundling in):
the separate "Try It" live-code-editor blocks (`docs/src/theme/ReactLiveScope`)
render components with no theme provider at all, not even light-mode Superset
theming -- `ReactLiveScope` only supplies a plain component scope for
`react-live`, not a wrapping provider. Fixing that means swizzling
`@docusaurus/theme-live-codeblock`'s `Playground`/`Preview` component, which is
a bigger, separate change.
### TESTING INSTRUCTIONS
Added
`superset-frontend/packages/superset-core/src/theme/SupersetThemeProvider.test.tsx`,
characterizing the mechanism this fix actually depends on: that calling
`toggleDarkMode()` on an already-mounted `Theme` instance, from *outside*
React's render cycle, correctly re-renders its context-consuming descendants
(i.e. real antd components, same as the docs demos). Caught a real, non-obvious
gotcha along the way and left it documented in a code comment: a probe reading
the theme object directly (rather than via antd's `useToken()` context hook)
misleadingly appears to *not* update, because React bails out of re-rendering a
child whose element reference is unchanged -- unless that child consumes a
Context whose value changed, which is exactly the antd `ConfigProvider` path
every real themed component uses.
```
PASS packages/superset-core/src/theme/SupersetThemeProvider.test.tsx
✓ an already-mounted SupersetThemeProvider re-renders context-consuming
children when toggleDarkMode is called on the same instance
✓ a toggleDarkMode call on an unmounted (or different) theme instance does
not affect a mounted provider
```
Also ran the full existing `packages/superset-core/src/theme` suite (97
tests) to confirm no regressions, and validated `StorybookWrapper.jsx` through
`eslint`/`prettier`/`tsc` via `pre-commit run`.
I wasn't able to get a full local visual check of the actual docs page
rendering (`yarn start` for `docs/` pulls in most of the frontend monorepo via
webpack aliases and OOM-killed repeatedly in my sandbox -- it's a genuinely
heavy build, per an existing comment in `webpack.extend.ts` about the same
barrel-import OOM risk). The test above verifies the actual propagation
mechanism directly rather than through a full page render, but a
screenshot/visual pass from CI or a reviewer on the deploy preview would still
be good confirmation before merge.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #40459
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
