codeant-ai-for-open-source[bot] commented on code in PR #40763:
URL: https://github.com/apache/superset/pull/40763#discussion_r3376278761
##########
superset-frontend/src/components/CrudThemeProvider.tsx:
##########
@@ -83,7 +91,7 @@ export default function CrudThemeProvider({
};
}, [dashboardTheme, fontUrls]);
- if (!dashboardTheme) {
+ if (!dashboardTheme || hasThemeConfigOverride) {
Review Comment:
You're right — the current change fixes the **provider precedence**, but it
does **not** fully suppress dashboard-theme side effects.
### Why the comment is correct
Even when `hasThemeConfigOverride` is `true`, `CrudThemeProvider` still:
- parses `theme.json_data`
- computes `fontUrls`
- runs the `useEffect` that injects `<style data-superset-fonts>`
So the dashboard theme can still leak artifacts (especially font imports)
even though its `SupersetThemeProvider` is skipped.
### Minimal fix
Guard the dashboard-theme work behind the override flag as well, so we avoid
parsing/injecting anything from the dashboard theme when SDK override mode is
active.
For example:
```tsx
const hasThemeConfigOverride = themeContext?.hasThemeConfigOverride ?? false;
const { dashboardTheme, fontUrls } = useMemo(() => {
if (hasThemeConfigOverride || !theme?.json_data) {
return { dashboardTheme: null, fontUrls: undefined };
}
// existing parsing logic
}, [theme, hasThemeConfigOverride]);
useEffect(() => {
if (hasThemeConfigOverride || !dashboardTheme || !fontUrls?.length) {
return;
}
// existing font injection logic
}, [dashboardTheme, fontUrls, hasThemeConfigOverride]);
```
And keep:
```tsx
if (!dashboardTheme || hasThemeConfigOverride) {
return <>{children}</>;
}
```
### Conclusion
So yes, the issue is valid: the override is currently **incomplete** because
dashboard font side effects still run. I’d recommend applying the guard in both
the parsing and injection paths to make the SDK override truly win.
If you want, I can also review the other comments on this PR and help
tighten those up too.
--
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]
