Skip to content

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Sep 13, 2025

SUMMARY

This PR improves the loading screen experience for users with dark mode preferences and enhances accessibility.

Changes:

  • Added CSS styles to support system dark mode preference (@media (prefers-color-scheme: dark))
  • Improved accessibility by using aria-label="Loading" instead of alt text for spinner images
  • Ensures loading indicators work properly in both light and dark themes

The changes provide a better initial loading experience that respects user's system theme preferences before the main application loads and eliminates jarring white flashes in dark mode.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2025-09-15.at.1.43.15.PM.mov

TESTING INSTRUCTIONS

  1. Set your system to dark mode preference
  2. Load a Superset page and observe the loading screen
  3. Verify the loading screen background/text matches your system theme
  4. Test with screen reader to confirm "Loading" is announced properly
  5. Verify no white flash occurs during page load in dark mode

ADDITIONAL INFORMATION

  • Changes UI

🤖 Generated with Claude Code

eschutho and others added 4 commits September 12, 2025 17:18
…filter sorting, and fix modal theming

- Remove top padding from horizontal filter bar to eliminate visual gap
- Set themed background color on loading screen to respect dark mode
- Remove fallback "Loading..." text to prevent Times New Roman display
- Use colorBgBase theme token for proper light/dark mode support
- Fix filter value sorting to preserve backend metric-based order when sortMetric is specified
- Make Modal.confirm theme-aware to respect current light/dark mode

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Add App wrapper to SupersetThemeProvider to enable theme inheritance for imperative modals
- Modal.confirm dialogs now properly inherit dark/light theme from ConfigProvider
- Simplifies theming approach by leveraging antd's built-in App context

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Use render_app_template() in EmbeddedView to provide proper template context
- Move tokens variable definition before its usage in spa.html template
- Fixes 'tokens is undefined' error in embedded dashboard views

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Fixes white flash in dark mode and removes fallback "Loading..." text
that was displaying in Times New Roman font.

Changes:
- Set themed background color on body using colorBgBase token
- Supports both light (#ffffff) and dark mode backgrounds
- Remove "Loading..." alt text from branded spinner image
- Remove fallback "Loading..." text div entirely
- Let React app handle loading state instead of HTML fallback

This eliminates the white background flash when loading in dark mode
and removes the unstyled "Loading..." text that appeared briefly.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I've completed my review and didn't find any issues.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@dosubot dosubot bot added change:frontend Requires changing the frontend global:theming Related to theming Superset labels Sep 13, 2025
@eschutho eschutho added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Sep 13, 2025
@github-actions github-actions bot added 🎪 6f6d925 🚦 building Environment 6f6d925 status: building 🎪 6f6d925 📅 2025-09-13T02-53 Environment 6f6d925 created at 2025-09-13T02-53 🎪 6f6d925 ⌛ 24h Environment 6f6d925 expires after 24h 🎪 6f6d925 🤡 eschutho Environment 6f6d925 requested by eschutho and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Sep 13, 2025
Copy link
Contributor

🎪 Showtime is building environment on GHA for 6f6d925

@github-actions github-actions bot added 🎪 6f6d925 🚦 deploying Environment 6f6d925 status: deploying 🎪 6f6d925 🚦 running Environment 6f6d925 status: running 🎪 🎯 6f6d925 Active environment pointer - 6f6d925 is receiving traffic 🎪 6f6d925 🌐 44.251.170.59:8080 Environment 6f6d925 URL: http://44.251.170.59:8080 (click to visit) and removed 🎪 6f6d925 🚦 building Environment 6f6d925 status: building 🎪 6f6d925 🚦 deploying Environment 6f6d925 status: deploying 🎪 6f6d925 🚦 running Environment 6f6d925 status: running 🎪 🎯 6f6d925 Active environment pointer - 6f6d925 is receiving traffic labels Sep 13, 2025
Copy link
Contributor

🎪 Showtime deployed environment on GHA for 6f6d925

Environment: http://44.251.170.59:8080 (admin/admin)
Lifetime: 24h auto-cleanup
Updates: New commits create fresh environments automatically

@eschutho eschutho marked this pull request as draft September 13, 2025 03:32
Added dark mode styling and improved loading indicators.
@yousoph yousoph added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Sep 18, 2025
@github-actions github-actions bot added 🎪 7e44735 🚦 building Environment 7e44735 status: building 🎪 7e44735 📅 2025-09-18T23-42 Environment 7e44735 created at 2025-09-18T23-42 🎪 7e44735 ⌛ 48h Environment 7e44735 expires after 48h and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Sep 18, 2025
Copy link
Contributor

🎪 Showtime is building environment on GHA for 7e44735

@github-actions github-actions bot added 🎪 7e44735 🤡 yousoph Environment 7e44735 requested by yousoph 🎪 7e44735 🚦 deploying Environment 7e44735 status: deploying 🎪 7e44735 🚦 running Environment 7e44735 status: running 🎪 🎯 7e44735 Active environment pointer - 7e44735 is receiving traffic 🎪 7e44735 🌐 35.87.124.232:8080 Environment 7e44735 URL: http://35.87.124.232:8080 (click to visit) and removed 🎪 7e44735 🚦 building Environment 7e44735 status: building 🎪 7e44735 🚦 deploying Environment 7e44735 status: deploying 🎪 7e44735 🚦 running Environment 7e44735 status: running 🎪 🎯 7e44735 Active environment pointer - 7e44735 is receiving traffic labels Sep 18, 2025
Copy link
Contributor

🎪 Showtime deployed environment on GHA for 7e44735

Environment: http://35.87.124.232:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@github-actions github-actions bot removed 🎪 7e44735 ⌛ 48h Environment 7e44735 expires after 48h 🎪 7e44735 📅 2025-09-18T23-42 Environment 7e44735 created at 2025-09-18T23-42 🎪 7e44735 🌐 35.87.124.232:8080 Environment 7e44735 URL: http://35.87.124.232:8080 (click to visit) 🎪 7e44735 🤡 yousoph Environment 7e44735 requested by yousoph 🎪 7e44735 🚦 running Environment 7e44735 status: running labels Sep 21, 2025
@eschutho eschutho changed the base branch from 6.0-bug-fixes to master September 24, 2025 22:00
@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 24, 2025
Copy link
Member

@msyavuz msyavuz left a comment

Choose a reason for hiding this comment

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

I think there is some cleanup needed

Comment on lines 376 to 377
error: AntdModal.error,
warning: AntdModal.warning,
Copy link
Member

Choose a reason for hiding this comment

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

Should these two use the themed version as well?

<GlobalStyles />
<ConfigProvider theme={themeState.antdConfig}>
{children}
<App>{children}</App>
Copy link
Member

Choose a reason for hiding this comment

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

This provides some reset styles that could mess up other parts of the ui.

Comment on lines +320 to +333
// When sortMetric is specified, the backend already sorted the data correctly
// Don't override the backend's metric-based sorting with frontend alphabetical sorting
if (formData.sortMetric) {
return 0; // Preserve the original order from the backend
}

// Only apply alphabetical sorting when no sortMetric is specified
const labelComparator = propertyComparator('label');
if (formData.sortAscending) {
return labelComparator(a, b);
}
return labelComparator(b, a);
},
[formData.sortAscending],
[formData.sortAscending, formData.sortMetric],
Copy link
Member

Choose a reason for hiding this comment

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

This looks like leftover code from another pr

Comment on lines -92 to -94
bootstrap_data=json.dumps(
bootstrap_data, default=json.pessimistic_json_iso_dttm_ser
),
Copy link
Member

Choose a reason for hiding this comment

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

Getting rid of this will make it so there will be errors when the serialization fails

@eschutho
Copy link
Member Author

I think Claude did a bad rebase.. will clean up.

Add aria-label to loading spinner image for screen reader accessibility while keeping empty alt text to prevent fallback display if image fails to load.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@eschutho eschutho force-pushed the fix/loading-screen-dark-mode-theming branch from 1a22cf8 to 3732966 Compare September 25, 2025 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:frontend Requires changing the frontend global:theming Related to theming Superset packages preset-io size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants