feat(config): add LOADERS variable in config files to enable custom loader gif
SUMMARY
Added a LOADERS variable in the configuration files config.py and superset_config.py:
- config.py default value → LOADERS = [{"src": "/static/assets/images/loading.gif"}]
- superset_config.py → LOADERS = [{"src": "/static/assets/images/YOUR_GIF.gif"}] (for exemple)
Managed the loader in Superset's base HTML skeleton:
- superset/superset/templates/superset/basic.html
Retrieved the custom loader in Superset API endpoints:
- superset/superset/views/core.py → Added an endpoint to retrieve the loader from the configuration file.
Managed the loader in Superset's React.js elements:
- superset/superset-frontend/src/components/Loading/index.tsx → Fetches the custom loader via the new endpoint.
I'm very new to Superset and the open-source world, and this is my first attempt. I'm open to any advice on how to make this cleaner if needed :)
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Don't setup the LOADERS in superset_config.py file to see the default loader. Then add tyour custom gif to see the difference.
ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
Codecov Report
Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
Project coverage is 83.75%. Comparing base (
76d897e) to head (9682fc5). Report is 1259 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| superset/views/core.py | 71.42% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #31565 +/- ##
===========================================
+ Coverage 60.48% 83.75% +23.26%
===========================================
Files 1931 538 -1393
Lines 76236 39135 -37101
Branches 8568 0 -8568
===========================================
- Hits 46114 32777 -13337
+ Misses 28017 6358 -21659
+ Partials 2105 0 -2105
| Flag | Coverage Δ | |
|---|---|---|
| hive | 48.76% <77.77%> (-0.41%) |
:arrow_down: |
| javascript | ? |
|
| mysql | 76.51% <77.77%> (?) |
|
| postgres | 76.57% <77.77%> (?) |
|
| presto | 53.27% <77.77%> (-0.54%) |
:arrow_down: |
| python | 83.75% <77.77%> (+20.26%) |
:arrow_up: |
| sqlite | 76.03% <77.77%> (?) |
|
| unit | 60.88% <77.77%> (+3.25%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@rusackas thank you for your feedback. Yes, I agree that it would be cleaner. I'll take care of it.
As for the array, I followed the same approach as for the FAVICONS. I probably didn’t quite understand why they were in array form as well?
Otherwise, for this new element, I saw a GitHub issue mentioning loader customization. This could serve as a basis in case we want to add keys like the positioning of the loader or other ideas.
@rusackas I noticed that the loader is not handled properly in an embedding context. The image does not load. Do you have any idea what might be causing this issue? I came across a topic that could potentially relate to my problem, but the lines referenced in config.py no longer seem relevant (https://github.com/apache/superset/issues/28112#issuecomment-2062595472) @mistercrunch.
It feels like all this should be included in the theme object eventually.
Heya... this hasn't been touched in a while, and has a bunch of CI issues, including pre-commit failures.
Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following:
pip3 install -r requirements/development.txt
pre-commit install
A series of checks will now run when you make a git commit.
Alternatively it is possible to run pre-commit by running pre-commit manually:
pre-commit run --all-files
I'm still not sure how to solve the embedded context issue, have you had any luck there? I suspect there needs to be a CSP adjustment made somewhere, but maybe you can see something in your browser logs?
Heya... this hasn't been touched in a while, and has a bunch of CI issues, including pre-commit failures.
Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following:
pip3 install -r requirements/development.txt pre-commit install A series of checks will now run when you make a git commit.Alternatively it is possible to run pre-commit by running pre-commit manually:
pre-commit run --all-filesI'm still not sure how to solve the embedded context issue, have you had any luck there? I suspect there needs to be a CSP adjustment made somewhere, but maybe you can see something in your browser logs?
Hey @rusackas, i don't understand cause i already use pre commit and i have this when i commit something:
Need a rebase on this one... but also I'm working on introducing a native SVG loader that's themable (as part of the newer Theming effort). I'll try to make sure we can fall back to a specified GIF (or whatever) if present.
Wanted to note that the theming branch now includes tokens for the logo img, positioning and scaling. I believe it's probably the right place to also reference a custom spinner. This is likely to conflict with the theming branch and break the inteferface. Would recommend targeting the theming branch of waiting for it to merge to open something that's cohesive with the work taking place there.
Here's a ref of how related things will be done in the future. I'd say we put this one on hold or try to target that branch. Note that there's a lot going on there and should merge into master soonish (few weeks at most (?))
https://github.com/apache/superset/blob/template_less/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx#L66-L70
Was talking with @ERGO1995 on Slack... we'll wait a bit for this one. I'll just mark it as Draft for now.
Closing this stale draft PR. It has been inactive for about 6 months and would need a rebase on master given the 12 months since it was opened.
I believe this functionality has largely been addressed by the Theming effort, but if there's more to do here, we welcome the discussion. Feel free to open an issue or fresh PR if you'd like to continue this work.
@ERGO1995 thanks for your contribution!