codeant-ai-for-open-source[bot] commented on code in PR #34629:
URL: https://github.com/apache/superset/pull/34629#discussion_r2733574786
##########
superset-frontend/packages/superset-core/src/ui/theme/Theme.tsx:
##########
@@ -174,18 +193,27 @@ export class Theme {
const [themeState, setThemeState] = React.useState({
theme: this.theme,
antdConfig: this.antdConfig,
- emotionCache: createCache({ key: 'superset' }),
+ emotionCache: this.createCache(),
});
+ const { direction = 'ltr' } = themeState.theme;
this.updateProviders = (theme, antdConfig, emotionCache) => {
setThemeState({ theme, antdConfig, emotionCache });
+ const dir = (theme && (theme as any).direction) ?? 'ltr';
+ // Ensure we update the document direction attribute on every change,
+ // and safely guard for non-browser environments.
+ if (typeof document !== 'undefined' && document.documentElement) {
+ document.documentElement.setAttribute('dir', dir);
+ document.documentElement.setAttribute('data-direction', dir);
+ document?.documentElement?.setAttribute('data-direction', 'rtl');
Review Comment:
**Suggestion:** Logic bug: an unconditional line sets `data-direction` to
the literal 'rtl', which will always override the intended `dir` value and
force the document into RTL mode even when the theme requests LTR. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Application layout forced into RTL sitewide.
- ❌ SupersetThemeProvider direction behavior broken.
- ❌ Emotion cache selection mismatch (ltr vs rtl).
- ⚠️ LanguagePicker-driven direction changes unreliable.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the frontend and render the theme provider: SupersetThemeProvider in
Theme.tsx at
line 187 (function SupersetThemeProvider). The provider mounts and defines
updateProviders
at Theme.tsx:200.
2. Trigger a theme direction change (for example by calling
Theme.setDirection(...) which
calls this.updateProviders) — setDirection invokes updateProviders with the
new theme
object (see Theme.setDirection which calls this.updateProviders;
updateProviders is
defined at Theme.tsx:200).
3. updateProviders executes: it calls setThemeState at Theme.tsx:201,
computes dir at
Theme.tsx:202, then sets document attributes at Theme.tsx:206-208. The final
unconditional
call at Theme.tsx:208 runs
document?.documentElement.setAttribute('data-direction',
'rtl').
4. Observe the document element attributes (e.g., in browser devtools
Elements tab):
despite passing direction 'ltr', data-direction becomes 'rtl' because the
hardcoded
setAttribute('data-direction', 'rtl') at Theme.tsx:208 overwrote the
intended value.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-core/src/ui/theme/Theme.tsx
**Line:** 208:208
**Comment:**
*Logic Error: Logic bug: an unconditional line sets `data-direction` to
the literal 'rtl', which will always override the intended `dir` value and
force the document into RTL mode even when the theme requests LTR.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
