codeant-ai-for-open-source[bot] commented on code in PR #36671:
URL: https://github.com/apache/superset/pull/36671#discussion_r2622808776
##########
superset-frontend/src/features/home/SavedQueries.tsx:
##########
@@ -348,25 +337,21 @@ export const SavedQueries = ({
)
}
actions={
- <QueryData>
- <ListViewCard.Actions
- onClick={e => {
- e.stopPropagation();
- e.preventDefault();
- }}
+ <ListViewCard.Actions
+ onClick={e => {
+ e.stopPropagation();
+ e.preventDefault();
+ }}
+ >
+ <Dropdown
+ menu={{ items: menuItems(q) }}
+ trigger={['click', 'hover']}
>
- <Dropdown
- menu={{
- items: menuItems(q),
- }}
- trigger={['click', 'hover']}
- >
- <Button buttonSize="xsmall" buttonStyle="link">
- <Icons.MoreOutlined iconColor={theme.colorText} />
- </Button>
- </Dropdown>
- </ListViewCard.Actions>
- </QueryData>
+ <Button buttonSize="xsmall" buttonStyle="link">
Review Comment:
**Suggestion:** The click handler that stops propagation and calls
preventDefault is attached to the container (`ListViewCard.Actions`), which
interferes with the Dropdown's trigger and may prevent the menu from opening;
move the event handling to the actual clickable Button and only stop
propagation (avoid preventDefault) so the Dropdown still receives/handles the
click as expected. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
<ListViewCard.Actions>
<Dropdown
menu={{ items: menuItems(q) }}
trigger={['click', 'hover']}
>
<Button
buttonSize="xsmall"
buttonStyle="link"
onClick={e => {
// Only stop propagation to avoid clicking the
card parent,
// but don't call preventDefault so Dropdown can
handle the event.
e.stopPropagation();
}}
>
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current parent-level handler calls preventDefault and stopPropagation
which can interfere with child handlers (the Dropdown's click trigger). Moving
the stopPropagation to the actual Button and dropping preventDefault is a
minimal, targeted fix that prevents the click from bubbling to the card while
allowing the Dropdown to process the click normally. This addresses a real
UX/logic issue and is safe to apply.
</details>
<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/SavedQueries.tsx
**Line:** 340:350
**Comment:**
*Logic Error: The click handler that stops propagation and calls
preventDefault is attached to the container (`ListViewCard.Actions`), which
interferes with the Dropdown's trigger and may prevent the menu from opening;
move the event handling to the actual clickable Button and only stop
propagation (avoid preventDefault) so the Dropdown still receives/handles the
click as expected.
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]