codeant-ai-for-open-source[bot] commented on code in PR #38399:
URL: https://github.com/apache/superset/pull/38399#discussion_r2884880622
##########
superset/templates/superset/spa.html:
##########
@@ -35,19 +35,35 @@
{% endblock %}
<style>
+ {% if entry == 'embedded' %}
+ html, body {
+ background: transparent !important;
+ }
+ {% else %}
body {
- background: #fff;
+ background: {{ theme_tokens.colorBgBase | default('#fff') }};
color: #000;
}
-
- @media (prefers-color-scheme: dark) {
- body {
- background: #000;
- color: #fff;
- }
- }
+ {% endif %}
</style>
+ {% if dark_theme_bg and entry != 'embedded' %}
+ <script nonce="{{ macros.get_nonce() }}">
+ (function() {
+ try {
+ var mode = localStorage.getItem('superset-theme-mode');
+ var shouldBeDark = mode === 'dark' ||
+ (mode !== 'default' && window.matchMedia('(prefers-color-scheme:
dark)').matches);
Review Comment:
**Suggestion:** The dark-mode detection logic treats any non-'default' mode
(including null/undefined) as "follow OS preference", so for first-time users
or cases where the theme mode isn't set, a dark OS will still result in a dark
background flash even though Superset's actual default theme is light,
reintroducing the original mismatch this PR aims to fix. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dark background flash for first-time dark-OS users.
- ⚠️ Initial paint theme mismatches Superset default light theme.
```
</details>
```suggestion
var mode = localStorage.getItem('superset-theme-mode');
var shouldBeDark = mode === 'dark' ||
(mode === 'system' && window.matchMedia('(prefers-color-scheme:
dark)').matches);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with this PR code and ensure the SPA template
`superset/templates/superset/spa.html` is used for the main UI entry.
2. In the browser, clear `localStorage['superset-theme-mode']` for the
Superset origin so
no explicit mode is stored (first-time user scenario).
3. Set the operating system / browser to dark mode and reload the main
Superset page
(non-embedded) so the inline script at `spa.html:54-56` executes before
paint.
4. Confirm via devtools console that `mode` is `null`, so `mode !==
'default'` is true and
`window.matchMedia('(prefers-color-scheme: dark)').matches` is true, making
`shouldBeDark`
true and injecting a dark background before the React app (whose default
theme is light)
renders, producing the dark flash this PR is intended to eliminate.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/templates/superset/spa.html
**Line:** 54:56
**Comment:**
*Logic Error: The dark-mode detection logic treats any non-'default'
mode (including null/undefined) as "follow OS preference", so for first-time
users or cases where the theme mode isn't set, a dark OS will still result in a
dark background flash even though Superset's actual default theme is light,
reintroducing the original mismatch this PR aims to fix.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38399&comment_hash=b797a06cd8d12e7cb18475f7e94bf29febd3b230a6c7506aca614da8138801e6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38399&comment_hash=b797a06cd8d12e7cb18475f7e94bf29febd3b230a6c7506aca614da8138801e6&reaction=dislike'>👎</a>
--
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]
