Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(config): add LOADERS variable in config files to enable custom loader gif #31565

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat(config): add LOADERS variable in config files to enable custom loader gif #31565

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6197113
add LOADERS variable in config and mecanic to get it in files
Dec 19, 2024
1112126
rm import React
Dec 20, 2024
b3243ca
update on index.tsx
Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions superset-frontend/src/components/Loading/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/

import React, { useEffect, useState } from 'react';

Check failure on line 19 in superset-frontend/src/components/Loading/index.tsx

View workflow job for this annotation

GitHub Actions / frontend-build 

Default React import is not required due to automatic JSX runtime in React 16.4
import { styled } from '@superset-ui/core';
import cls from 'classnames';
import Loader from 'src/assets/images/loading.gif';

export type PositionOption =
| 'floating'
Expand Down Expand Up @@ -61,6 +60,19 @@
image,
className,
}: Props) {
const [Loader, setLoader] = useState<string | undefined>(undefined);
useEffect(() => {
fetch('/superset/loaders/')
.then(response => response.json())
.then(data => {
if (data && Array.isArray(data) && data[0]?.src) {
setLoader(data[0].src);
}
})
.catch(() => {
setLoader(undefined);
});
}, []);
return (
<LoaderImg
className={cls('loading', position, className)}
Expand Down
1 change: 1 addition & 0 deletions superset/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
# },
FAVICONS = [{"href": "/static/assets/images/favicon.png"}]

LOADERS = [{"src": "/static/assets/images/loading.gif"}]
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this is a bit of a side mission for another PR, or it could be part of this, but I notice we have the FAVICON and LOADERS up here. APP_ICON is waaaaay down below, but maybe they should live together in the config?

LOADERS could also probably use a comment explaining what it is, for anyone learning what they can config.

Is there a reason it's an array? Can you use more than one? If it's just a single GIF, it should probably be more singular like APP_ICON. And to be crazy pedantic, maybe it should just be "LOADER" rather than "LOADERS"


def _try_json_readversion(filepath: str) -> str | None:
try:
Expand Down
7 changes: 5 additions & 2 deletions superset/templates/superset/basic.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
{% import 'appbuilder/general/lib.html' as lib %} {% from
'superset/partials/asset_bundle.html' import css_bundle, js_bundle with context
%} {% set favicons = appbuilder.app.config['FAVICONS'] %}
{% set loaders = appbuilder.app.config['LOADERS'] %}
{% import "superset/macros.html" as macros %}
<html>
<head>
Expand Down Expand Up @@ -74,9 +75,10 @@
<body {% if standalone_mode %}class="standalone" {% endif %}>
{% block navbar %} {% if not standalone_mode %} {% include
'appbuilder/navbar.html' %} {% endif %} {% endblock %} {% block body %}
<div id="app" data-bootstrap="{{ bootstrap_data }}">
<div id="app" data-bootstrap="{{ bootstrap_data }}" {% if standalone_mode %} {% endif %}>
{% for loader in loaders %}
<img
src="{{ assets_prefix }}/static/assets/images/loading.gif"
src="{{ assets_prefix }}{{loader.src}}"
style="
width: 50px;
position: absolute;
Expand All @@ -85,6 +87,7 @@
transform: translate(-50%, -50%);
"
/>
{% endfor %}
</div>
{% endblock %}

Expand Down
9 changes: 9 additions & 0 deletions superset/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,3 +925,12 @@
@deprecated(new_target="/sqllab/history")
def sqllab_history(self) -> FlaskResponse:
return redirect("/sqllab/history")

@api
@expose("/loaders/", methods=("GET",))
def get_loaders(self) -> FlaskResponse:
"""
Expose the custom LOADERS configuration via an API endpoint.
"""
loaders = app.config.get("LOADERS", [])
return loaders

Check warning on line 936 in superset/views/core.py

View check run for this annotation

Codecov / codecov/patch

superset/views/core.py#L935-L936 

Added lines #L935 - L936 were not covered by tests
Loading