You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/18 05:43:47 UTC

[PR] fix(menu): highlight active nav tab in non-English locales [superset]

rusackas opened a new pull request, #41183:
URL: https://github.com/apache/superset/pull/41183

   ### SUMMARY
   
   The top navigation bar highlights the active tab by matching the current route against the menu item keys. Those keys were the menu items' **displayed labels**, while the active-tab matcher compared against **hardcoded English strings** (`"Dashboards"`, `"Charts"`, `"SQL"`, …). In any non-English locale the displayed label is translated, so it never matched the English comparison string and **no tab was highlighted**.
   
   This is a systemic i18n bug — it affects every language whose menu translations differ from English, not just Russian (reported in #36403).
   
   **Fix:** key each menu item by its stable Flask-AppBuilder `name` (the internal view/category identifier, which is locale-independent) instead of its localized label, and match the active tab against those same names. `name` falls back to `label` when absent, so operator-defined custom menu entries keep working.
   
   The change is in `superset-frontend/src/features/home/Menu.tsx`:
   - `buildMenuItem` now uses `key: name ?? label` for top-level entries.
   - The route → active-tab effect sets the stable names (`Dashboards`, `Charts`, `Datasets`, `SQL Lab`) instead of English labels. As a bonus this decouples the Datasets tab from the semantic-layer label override.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Before — Russian locale, on the Dashboards page, the **Dashboards** tab is not highlighted.
   After — the active tab is highlighted in every language.
   
   (See the screenshots in #36403 for the broken-vs-working comparison.)
   
   ### TESTING INSTRUCTIONS
   
   1. Set two languages in `superset_config.py`:
      ```py
      LANGUAGES = {
          "en": {"flag": "us", "name": "English"},
          "ru": {"flag": "ru", "name": "Russian"},
      }
      ```
   2. Switch the UI language to Russian.
   3. Open the Dashboards (or Charts / Datasets / SQL Lab) page.
   4. Confirm the corresponding top-nav tab is highlighted as active.
   
   Automated coverage added in `Menu.test.tsx`:
   - Highlights the active top-level tab on a matching route (English) — regression guard.
   - Highlights the active top-level tab when the label is **localized**.
   - Highlights the active SQL submenu tab when the label is localized.
   
   ```
   npx jest src/features/home/Menu.test.tsx
   ```
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: Fixes #36403
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41183:
URL: https://github.com/apache/superset/pull/41183


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4738704944

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41183?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `66.66667%` with `2 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 64.31%. Comparing base ([`ae0b1f0`](https://app.codecov.io/gh/apache/superset/commit/ae0b1f0308af08ea706bca196c40ba186b2c3daa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`27c0736`](https://app.codecov.io/gh/apache/superset/commit/27c07367f711d028ca9d73d6dd1f09036c24483d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/41183?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset-frontend/src/features/home/Menu.tsx](https://app.codecov.io/gh/apache/superset/pull/41183?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Ffeatures%2Fhome%2FMenu.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVzL2hvbWUvTWVudS50c3g=) | 66.66% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #41183   +/-   ##
   =======================================
     Coverage   64.31%   64.31%           
   =======================================
     Files        2651     2651           
     Lines      144773   144775    +2     
     Branches    33407    33409    +2     
   =======================================
   + Hits        93111    93118    +7     
   + Misses      49992    49987    -5     
     Partials     1670     1670           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/41183/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/41183/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.46% <66.66%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41183?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4740353402

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4804400838

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #41183:
URL: https://github.com/apache/superset/pull/41183#discussion_r3476331889


##########
superset-frontend/src/features/home/Menu.test.tsx:
##########
@@ -796,3 +796,105 @@ test('brand link falls back to brand.path when theme brandLogoUrl is absent', as
   // ensureAppRoot must have been applied: /welcome/ → /superset/welcome/
   expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
 });
+
+// --- Active tab highlighting (regression tests for issue #36403) ---
+//
+// The active top-level tab is highlighted by matching the current route to a
+// menu item. The matching must rely on a stable identifier (the FAB `name`),
+// not the displayed label, otherwise highlighting breaks for any non-English
+// locale where the label is translated.
+
+// Returns the top-level <li> that contains the given visible text, so we can
+// assert whether antd marked it as the selected menu item.
+const getMenuItemByText = (text: string): HTMLElement | null =>
+  screen.getByText(text).closest('li');
+
+afterEach(() => {

Review Comment:
   MEDIUM: `afterEach` declared at module scope (not inside a `describe`) applies to **all** tests in `Menu.test.tsx` — including the ~798 lines of existing tests above it — not just the three new ones. The path reset is benign today, but future test authors won't expect a file-level cleanup hook buried at line 812, and any test that checks state after navigation could be silently affected.
   
   Wrap the three new `test()` blocks (and this `afterEach` + `getMenuItemByText`) inside a `describe('active tab highlighting (regression #36403)', () => { … })` so the cleanup is scoped to only these tests.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41183:
URL: https://github.com/apache/superset/pull/41183#discussion_r3433394462


##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -242,10 +252,14 @@ export function Menu({
     childs,
     url,
     isFrontendRoute,
+    name,
   }: MenuObjectProps): MenuItem => {
+    // Key items by the stable FAB `name` so active-tab matching is independent
+    // of the localized label. Fall back to the label when no name is provided.
+    const key = name ?? label;

Review Comment:
   **Suggestion:** Using `name ?? label` as the top-level key introduces a key collision for SQL Lab: the parent category key becomes `SQL Lab` while a child item key is also `SQL Lab` (child keys are still derived from `child.label`). Ant Design menu keys must be unique, otherwise selection/open-state behavior becomes inconsistent and active highlighting can target the wrong node. Generate guaranteed-unique keys across levels (for example, namespace parent/child keys or stop keying children by label alone). [missing react key]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ SQL Lab nav parent and child share identical menu key.
   - ⚠️ AntD MainNav selection state ambiguous for SQL Lab entries.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. On backend initialization, the SQL Lab menu entries are registered in
   `superset/initialization/__init__.py:11-27` via `appbuilder.add_link()`: the SQL Editor
   link is created with `category="SQL Lab"` and `category_label=_("SQL")` (top-level menu
   group), and `label=_("SQL Lab")` for the child item
   (`superset/initialization/__init__.py:11-19`), meaning the category internal name is `"SQL
   Lab"` while the first child's visible label is also `"SQL Lab"`.
   
   2. The top-level menu data is exposed to the frontend by `menu_data()` in
   `superset/views/base.py:14-28`, which returns `{"menu": appbuilder.menu.get_data(), ...}`;
   this structure contains a parent menu item `{ name: "SQL Lab", label: "SQL", childs: [...]
   }` and a first child `{ name: "SQL Editor", label: "SQL Lab", url: "/sqllab/" }` based on
   the configuration in step 1.
   
   3. In the SPA shell, `superset-frontend/src/views/App.tsx:62-70` reads
   `bootstrapData.common.menu_data` and renders the top navigation `<Menu
   data={bootstrapData.common.menu_data} isFrontendRoute={isFrontendRoute} />`, so every page
   load passes the SQL Lab parent and child entries described above into
   `superset-frontend/src/features/home/Menu.tsx`.
   
   4. Inside `buildMenuItem` in `superset-frontend/src/features/home/Menu.tsx:250-305`, the
   parent menu item's key is computed as `const key = name ?? label;` (`line 259`), so the
   SQL Lab parent gets `key === "SQL Lab"` from its `name`, while child items are still
   created with `key: \`\${child.label}\`` at `Menu.tsx:283-285`; for the SQL Editor child
   whose `label` is `"SQL Lab"`, this yields a second item with `key === "SQL Lab"`, creating
   a parent/child key collision.
   
   5. The rendered navigation uses `MainNav` from
   `superset-frontend/packages/superset-ui-core/src/components/Menu/index.tsx:162-174`, which
   is a styled wrapper around Ant Design's `Menu` (`import { Menu as AntdMenu } from 'antd';`
   at `Menu/index.tsx:21`), and AntD expects menu `key` values to be unique. With both the
   SQL Lab parent and its first child sharing `key: "SQL Lab"`, selection and open-state
   bookkeeping inside AntD's menu tree become ambiguous, which can manifest as warnings in
   the console and unstable highlighting/selection behavior for SQL Lab-related items.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9bff234c1e184e9d97b43d1109ddf972&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9bff234c1e184e9d97b43d1109ddf972&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/features/home/Menu.tsx
   **Line:** 259:259
   **Comment:**
   	*Missing React Key: Using `name ?? label` as the top-level key introduces a key collision for SQL Lab: the parent category key becomes `SQL Lab` while a child item key is also `SQL Lab` (child keys are still derived from `child.label`). Ant Design menu keys must be unique, otherwise selection/open-state behavior becomes inconsistent and active highlighting can target the wrong node. Generate guaranteed-unique keys across levels (for example, namespace parent/child keys or stop keying children by label alone).
   
   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%2F41183&comment_hash=6131c6bcef8c42c54a70ed131789d6d5d87c5ac1ecc7fe0e0061f6f9349f6203&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41183&comment_hash=6131c6bcef8c42c54a70ed131789d6d5d87c5ac1ecc7fe0e0061f6f9349f6203&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #41183:
URL: https://github.com/apache/superset/pull/41183#discussion_r3433538187


##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -242,10 +252,14 @@ export function Menu({
     childs,
     url,
     isFrontendRoute,
+    name,
   }: MenuObjectProps): MenuItem => {
+    // Key items by the stable FAB `name` so active-tab matching is independent
+    // of the localized label. Fall back to the label when no name is provided.
+    const key = name ?? label;

Review Comment:
   Good catch — the `SQL Editor` child labeled `SQL Lab` does collide with the `SQL Lab` parent key. Keyed children by their stable `name` too (fallback to label), so that pair no longer clashes.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4738447640

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4738479255

   <!-- Bito Reply -->
   The flagged issue regarding key collisions in the navigation menu is correct. Using `name ?? label` as the key causes a collision when a parent menu item's `name` matches a child item's `label` (e.g., 'SQL Lab').
   
   To resolve this, you should ensure keys are unique by namespacing them, for example by incorporating the parent's name or a unique identifier into the child's key. In `superset-frontend/src/features/home/Menu.tsx`, you can update the `buildMenuItem` function to generate a unique key for children, such as `key: parentName ? `${parentName}-${key}` : key`.
   
   I have checked the other comments on this PR, and there are no additional actionable review comments to address. Would you like me to proceed with implementing this fix in `Menu.tsx`?
   
   **superset-frontend/src/features/home/Menu.tsx**
   ```
   }: MenuObjectProps): MenuItem => {
       // Key items by the stable FAB `name` so active-tab matching is independent
       // of the localized label. Fall back to the label when no name is provided.
       const key = name ?? label;
       if (url && isFrontendRoute) {
         return {
           key: parentName ? `${parentName}-${key}` : key,
           label: (
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41183:
URL: https://github.com/apache/superset/pull/41183#issuecomment-4738455831

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | e27418c926cf21953ae66ed07d5e4b2c9ccca949 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a3385939055ee00084be1de |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41183--superset-docs-preview.netlify.app](https://deploy-preview-41183--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQxMTgzLS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.rfsZTa2lcA675WTVUH31A3g_tdfgEo3n1O3iCC_SqvQ)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpbCoAgEEDRvcx30sSoVDtoGT6mFCwlCnrQ3vPjwv04L5Q9r-WAEaABu5vNhfpzvFrSEkmk7EyKD3ux8naKEJeQakfVnkvK9-Sr14aoVwMNqBQzIvbScucZvh8) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-41183--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(menu): highlight active nav tab in non-English locales [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #41183:
URL: https://github.com/apache/superset/pull/41183#discussion_r3476797272


##########
superset-frontend/src/features/home/Menu.test.tsx:
##########
@@ -796,3 +796,105 @@ test('brand link falls back to brand.path when theme brandLogoUrl is absent', as
   // ensureAppRoot must have been applied: /welcome/ → /superset/welcome/
   expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
 });
+
+// --- Active tab highlighting (regression tests for issue #36403) ---
+//
+// The active top-level tab is highlighted by matching the current route to a
+// menu item. The matching must rely on a stable identifier (the FAB `name`),
+// not the displayed label, otherwise highlighting breaks for any non-English
+// locale where the label is translated.
+
+// Returns the top-level <li> that contains the given visible text, so we can
+// assert whether antd marked it as the selected menu item.
+const getMenuItemByText = (text: string): HTMLElement | null =>
+  screen.getByText(text).closest('li');
+
+afterEach(() => {

Review Comment:
   Good call — wrapped the three tests and their `afterEach` in a `describe` so the route reset stays scoped to them.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org