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>
[](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)
[](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: [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]
