fix(config): Wire LOGO_TARGET_PATH and document custom spinner usage#36951
fix(config): Wire LOGO_TARGET_PATH and document custom spinner usage#36951shashbha14 wants to merge 4 commits into
Conversation
Code Review Agent Run #3272beActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Looks simple enough, but can you perhaps add a simple test where you add a LOGO_TARGET_PATH and assert that it shows up in the code? |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the LOGO_TARGET_PATH configuration variable was ignored by the default theme system, and improves documentation around custom loading icons and logo configuration.
Changes:
- Fixed
LOGO_TARGET_PATHto be properly wired intoTHEME_DEFAULT["token"]["brandLogoHref"] - Documented the
brandSpinnerUrlconfiguration option for custom loading icons - Added clarifying comments about how
APP_ICON,LOGO_TARGET_PATH,LOGO_TOOLTIP, andLOGO_RIGHT_TEXTinteract with the theme system
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36951 +/- ##
===========================================
+ Coverage 0 66.44% +66.44%
===========================================
Files 0 668 +668
Lines 0 51391 +51391
Branches 0 5794 +5794
===========================================
+ Hits 0 34148 +34148
- Misses 0 15855 +15855
- Partials 0 1388 +1388
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Adds a unit test to verify that LOGO_TARGET_PATH is properly wired into THEME_DEFAULT["token"]["brandLogoHref"], as requested in review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
I've added a test to help see this through. |
Code Review Agent Run #570c3fActionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
This PR addresses user confusion regarding customizing the loading icon and resolves a bug where
LOGO_TARGET_PATHwas being ignored by the default theme system in Superset 6.0.0.Specifically, it:
brandSpinnerUrlinTHEME_DEFAULTwithin config.py. This clarifies that users can set a custom GIF/image loader via config without needing to replace source files or rebuild assets.THEME_DEFAULTto correctly useLOGO_TARGET_PATHforbrandLogoHref. Previously,LOGO_TARGET_PATHwas ignored by the default theme, forcing users to override the entire theme object just to change the logo link.APP_ICON,LOGO_TOOLTIP,LOGO_RIGHT_TEXT, andAPP_ICON_WIDTH(deprecation note) to clarify their interaction withTHEME_DEFAULTor their deprecated status.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (Configuration and Documentation changes only)
TESTING INSTRUCTIONS
To verify these changes locally:
Verify Logo Link Fix:
superset_config.py, setLOGO_TARGET_PATH = "https://custom.url".https://custom.url(previously it would stay on/unless the theme was overridden).Verify Custom Spinner:
superset_config.py, set:ADDITIONAL INFORMATION