codeant-ai-for-open-source[bot] commented on code in PR #34629:
URL: https://github.com/apache/superset/pull/34629#discussion_r3479719201
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -703,6 +718,15 @@ export class ThemeController {
};
}
+ /**
+ * Applies text direction from the bootstrap locale before the UI renders.
+ */
+ private initializeDirectionFromLocale(): void {
+ this.globalTheme.setDirection(
+ getDirectionFromLocale(getBootstrapLocale()),
+ );
Review Comment:
**Suggestion:** This new initialization path relies on
`getDirectionFromLocale(getBootstrapLocale())`, but locale parsing only strips
`-` regions, not `_` variants that are already used in language keys (for
example `fa_IR`/`ar_EG`). In those cases, RTL locales are misclassified as LTR
at startup. Normalize both hyphenated and underscored locale formats before
deriving direction. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ RTL locales with underscores render UI in LTR direction.
- ⚠️ Global theme direction inconsistent across configured locales.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset so the bootstrap locale uses an underscore-formatted
RTL code (for
example by setting the Python-side `LANGUAGES` mapping to emit
`common.locale = 'fa_IR'`
or `common.menu_data.navbar_right.locale = 'fa_IR'`), which is reflected
into the frontend
bootstrap data typed as `CommonBootstrapData.locale` and
`MenuData.navbar_right.locale` in
`superset-frontend/src/types/bootstrapTypes.ts:159-170`. This results in the
`data-bootstrap` payload containing an RTL locale with an underscore rather
than a hyphen.
2. Start the main UI so `RootContextProviders` instantiates the global
`ThemeController`
(`superset-frontend/src/views/RootContextProviders.tsx:5`) and provides it
via
`SupersetThemeProvider` (`RootContextProviders.tsx:16`), as well as the
embedded context
where `EmbeddedContextProviders` creates a separate `ThemeController`
(`superset-frontend/src/embedded/EmbeddedContextProviders.tsx:5-8`) that is
also wrapped
in `SupersetThemeProvider` at `EmbeddedContextProviders.tsx:22-49`.
3. In the `ThemeController` constructor
(`superset-frontend/src/theme/ThemeController.ts:117-182`), after applying
the initial
theme, it calls `this.initializeDirectionFromLocale();` at
`ThemeController.ts:180`. The
private method `initializeDirectionFromLocale()` at
`ThemeController.ts:721-728` computes
the text direction via
`this.globalTheme.setDirection(getDirectionFromLocale(getBootstrapLocale()));`.
`getBootstrapLocale()` in `superset-frontend/src/utils/localeUtils.ts:33-35`
returns the
underscore-formatted locale string from bootstrap (e.g. `'fa_IR'`), and
`getDirectionFromLocale()` in `localeUtils.ts:28-31` derives a language code
using
`getLanguageCodeFromLocale(locale)` which currently does
`locale.split('-')[0]`
(`localeUtils.ts:24-25`).
4. Because `getLanguageCodeFromLocale()` only splits on `'-'`, a locale like
`'fa_IR'` is
not reduced to `'fa'` but stays `'fa_IR'`, which is not present in the RTL
language list
defined in `superset-frontend/src/constants.ts:86-97`. As a result,
`getDirectionFromLocale('fa_IR')` returns `'ltr'` instead of `'rtl'`, and
`initializeDirectionFromLocale()` sets the global theme direction to LTR at
startup for
this RTL locale. Any subsequent calls to
`setDirection(getDirectionFromLocale(locale))`
from the language picker
(`superset-frontend/src/features/home/LanguagePicker.tsx:58-67`)
with the same underscore-formatted locale preserve the incorrect LTR
direction, so RTL
users with underscore-formatted locale codes see the entire UI initialized
and rendered in
the wrong direction.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f31448257c44d089edf36b1a9dacc16&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1f31448257c44d089edf36b1a9dacc16&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/theme/ThemeController.ts
**Line:** 725:727
**Comment:**
*Logic Error: This new initialization path relies on
`getDirectionFromLocale(getBootstrapLocale())`, but locale parsing only strips
`-` regions, not `_` variants that are already used in language keys (for
example `fa_IR`/`ar_EG`). In those cases, RTL locales are misclassified as LTR
at startup. Normalize both hyphenated and underscored locale formats before
deriving direction.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34629&comment_hash=d524a497114cdd45088a8b18c1d98395bed5d5dff1b2fe259008505e432cd8e9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34629&comment_hash=d524a497114cdd45088a8b18c1d98395bed5d5dff1b2fe259008505e432cd8e9&reaction=dislike'>👎</a>
--
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]
