codeant-ai-for-open-source[bot] commented on code in PR #38732:
URL: https://github.com/apache/superset/pull/38732#discussion_r2972052798
##########
superset-frontend/packages/superset-ui-core/src/components/Menu/index.tsx:
##########
@@ -84,35 +84,45 @@ const StyledMenu = styled(AntdMenu)`
const StyledNav = styled(AntdMenu)`
${({ theme }) => css`
- display: flex;
- align-items: center;
- height: 100%;
- gap: 0;
border-bottom: 0;
line-height: ${theme.lineHeight};
- &.ant-menu-horizontal > .ant-menu-item {
- height: 100%;
+
+ &.ant-menu-horizontal {
display: flex;
align-items: center;
- margin: 0;
- padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
- ::after {
- content: '';
- position: absolute;
- width: 98%;
- height: 2px;
- background-color: ${theme.colorPrimaryBorderHover};
- bottom: ${theme.sizeUnit / 4}px;
- left: 0;
- transform: scale(0);
- transition: 0.2s all ease-out;
+ height: 100%;
+ gap: 0;
+
+ > .ant-menu-item {
+ height: 100%;
+ display: flex;
+ align-items: center;
+ margin: 0;
+ padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+ ::after {
Review Comment:
**Suggestion:** The pseudo-element selector is missing `&`, so the rule
targets descendant pseudo-elements instead of the menu item itself. As a
result, the underline indicator may never be created on the actual menu item,
and selected-state styling that depends on that pseudo-element will not render
correctly. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Top navbar underline feedback missing on desktop navigation.
- ⚠️ Active tab highlight in `navbar-top` appears inconsistent.
```
</details>
```suggestion
&::after {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any SPA page that mounts global nav; `src/views/App.tsx:78-81`
renders `<Menu
data={...} />` from `src/features/home/Menu`.
2. In `src/features/home/Menu.tsx:353-359` (`StyledMainNav` render), desktop
breakpoint
uses `mode={screens.md ? 'horizontal' : 'inline'}`, so horizontal MainNav
styles are
applied on common desktop widths.
3. Styling comes from
`superset-frontend/packages/superset-ui-core/src/components/Menu/index.tsx:96-119`;
rule
at `:102` is `::after` inside `> .ant-menu-item`, which targets descendant
pseudo-elements, while selected state at `:118` targets
`.ant-menu-item-selected::after`
on the item itself.
4. Hover/select top-level navbar items (`data-test="navbar-top"`). Underline
does not
render on the menu item because the pseudo-element definition is not
attached to the same
selector as the selected-state transform.
```
</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-ui-core/src/components/Menu/index.tsx
**Line:** 102:102
**Comment:**
*Logic Error: The pseudo-element selector is missing `&`, so the rule
targets descendant pseudo-elements instead of the menu item itself. As a
result, the underline indicator may never be created on the actual menu item,
and selected-state styling that depends on that pseudo-element will not render
correctly.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38732&comment_hash=3211be9ed3a584a9e9cac33a09b2a0d313768679f61de24324401dd5b0cc3f6a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38732&comment_hash=3211be9ed3a584a9e9cac33a09b2a0d313768679f61de24324401dd5b0cc3f6a&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/components/Menu/index.tsx:
##########
@@ -84,35 +84,45 @@ const StyledMenu = styled(AntdMenu)`
const StyledNav = styled(AntdMenu)`
${({ theme }) => css`
- display: flex;
- align-items: center;
- height: 100%;
- gap: 0;
border-bottom: 0;
line-height: ${theme.lineHeight};
- &.ant-menu-horizontal > .ant-menu-item {
- height: 100%;
+
+ &.ant-menu-horizontal {
display: flex;
align-items: center;
- margin: 0;
- padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
- ::after {
- content: '';
- position: absolute;
- width: 98%;
- height: 2px;
- background-color: ${theme.colorPrimaryBorderHover};
- bottom: ${theme.sizeUnit / 4}px;
- left: 0;
- transform: scale(0);
- transition: 0.2s all ease-out;
+ height: 100%;
+ gap: 0;
+
+ > .ant-menu-item {
+ height: 100%;
+ display: flex;
+ align-items: center;
+ margin: 0;
+ padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+ ::after {
+ content: '';
+ position: absolute;
+ width: 98%;
+ height: 2px;
+ background-color: ${theme.colorPrimaryBorderHover};
+ bottom: ${theme.sizeUnit / 4}px;
+ left: 0;
+ transform: scale(0);
+ transition: 0.2s all ease-out;
+ }
+ :hover::after {
Review Comment:
**Suggestion:** The hover selector is missing `&`, which changes it into a
descendant hover selector rather than applying hover state to the menu item
itself. This breaks the intended underline animation when hovering a top-level
navigation item. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Hover underline animation missing for top-level navbar items.
- ⚠️ Desktop main-nav affordance weaker than submenu affordance.
```
</details>
```suggestion
&:hover::after {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load the main application shell where navbar is always rendered
(`src/views/App.tsx:78-81`) and ensure desktop breakpoint (`screens.md`) so
`StyledMainNav` is horizontal (`src/features/home/Menu.tsx:353-355`).
2. Hover a top-level item in the main navigation (`StyledMainNav` with
`data-test="navbar-top"` at `src/features/home/Menu.tsx:356`).
3. In
`superset-frontend/packages/superset-ui-core/src/components/Menu/index.tsx:113-115`,
selector is `:hover::after` (descendant hover selector), not
`&:hover::after` for the menu
item itself.
4. Compare with established working pattern in
`src/features/home/Menu.tsx:135-137`
(`&:hover::after` on submenu): hover underline animates there, but not on
top-level
`MainNav.Item` due to missing `&`.
```
</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-ui-core/src/components/Menu/index.tsx
**Line:** 113:113
**Comment:**
*Possible Bug: The hover selector is missing `&`, which changes it into
a descendant hover selector rather than applying hover state to the menu item
itself. This breaks the intended underline animation when hovering a top-level
navigation item.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38732&comment_hash=f841ff0b326395cec62c4835bf67e910ad57f7266d84e8fb61b341c39bfeb6cb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38732&comment_hash=f841ff0b326395cec62c4835bf67e910ad57f7266d84e8fb61b341c39bfeb6cb&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]
