martyngigg commented on code in PR #34935:
URL: https://github.com/apache/superset/pull/34935#discussion_r2329457536
##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -285,7 +286,7 @@ export function Menu({
>
<Image
preview={false}
- src={theme.brandLogoUrl}
+ src={assetUrl(theme.brandLogoUrl)}
Review Comment:
Maybe I have misunderstood how this now works with the new theming?
When `assetUrl` was originally implemented in #30134 it was assumed that
[`STATIC_ASSETS_PREFIX`](https://github.com/apache/superset/blob/0fce5ecfa5295f9978f873f2930ca972b868cfe5/superset/config.py#L1940)
would be a prefix to all static assets and if using a CDN for example then
this would be set as the `STATIC_ASSETS_PREFIX` and this would then become a
prefix to everything.
Similarly we have
[`ensureAppRoot`](https://github.com/apache/superset/blob/0fce5ecfa5295f9978f873f2930ca972b868cfe5/superset-frontend/src/features/home/RightMenu.tsx#L319)
for prefixing non-asset paths.
I think I saw that the default `theme.brandLogoUrl` value was a relative
path much like `superset_config[APP_ICON]` so I assumed the same rules applied.
Maybe `assetUrl()` could also check the value passed to it is a relative path
and only add the prefix then?
--
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]
