codeant-ai-for-open-source[bot] commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3493549640
##########
superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:
##########
@@ -93,6 +96,113 @@ const TabTitle = styled.span`
// Get the user's OS
const userOS = detectOS();
+const newTabTooltip =
+ userOS === 'Windows' ? t('New tab (Ctrl + q)') : t('New tab (Ctrl + t)');
+
+const PlusIcon = (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ vertical-align: middle;
+ `}
+ data-test="add-tab-icon"
+ />
+);
+
+function NewTabButton({ onAddSqlEditor }: { onAddSqlEditor: () => void }) {
+ const [open, setOpen] = useState(false);
+
+ const dropdownItems = useMemo<MenuItemType[]>(() => {
+ if (!open) return [];
+ const primaryItems =
+ menus.getMenu(ViewLocations.sqllab.newTab)?.primary ?? [];
+ return [
+ {
+ key: 'sql-editor',
+ label: t('SQL Editor'),
+ icon: <Icons.TableOutlined iconSize="m" />,
+ onClick: () => {
+ setOpen(false);
+ onAddSqlEditor();
+ },
+ },
+ ...primaryItems.flatMap(item => {
+ const command = commands.getCommand(item.command);
+ if (!command) {
+ // An extension contributed this menu item but its command isn't
+ // registered (load is still pending or failed). Skip it so clicking
+ // can't throw "Command not found" and break the add-tab flow.
+ return [];
+ }
+ const Icon = command.icon
+ ? ((Icons as Record<string, typeof Icons.FileOutlined>)[
+ command.icon
+ ] ?? Icons.FileOutlined)
+ : Icons.FileOutlined;
+ return [
+ {
+ key: command.id,
+ label: command.title ?? item.command,
+ icon: <Icon iconSize="m" />,
+ onClick: () => {
+ setOpen(false);
+ commands.executeCommand(item.command);
+ },
+ } as MenuItemType,
+ ];
+ }),
+ ];
+ }, [open, onAddSqlEditor]);
+
+ const activate = () => {
+ const primaryItems =
+ menus.getMenu(ViewLocations.sqllab.newTab)?.primary ?? [];
+ if (primaryItems.length === 0) {
+ onAddSqlEditor();
+ } else {
+ setOpen(prev => !prev);
+ }
+ };
+
+ const handleClick = (e: React.MouseEvent) => {
+ // Antd's Tabs wraps addIcon in its own <button onClick={() =>
onEdit('add')}>.
+ // Stop propagation so antd doesn't also call newQueryEditor() while we
handle it.
+ e.stopPropagation();
+ activate();
+ };
+
+ const handleKeyDown = (e: React.KeyboardEvent) => {
+ // The same wrapper button activates on Enter/Space; intercept those keys
so
+ // keyboard users reach the extension new-tab dropdown rather than antd's
+ // default add-tab path.
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ e.stopPropagation();
+ activate();
+ }
+ };
+
+ return (
+ <Tooltip id="add-tab" placement="left" title={newTabTooltip}>
+ <Dropdown
+ open={open}
+ onOpenChange={setOpen}
+ menu={{ items: dropdownItems }}
+ trigger={[]}
+ >
+ <span
+ role="button"
+ tabIndex={0}
+ onClick={handleClick}
+ onKeyDown={handleKeyDown}
+ >
+ {PlusIcon}
+ </span>
Review Comment:
**Suggestion:** The Enter/Space interception is attached to an inner span,
but AntD wraps addIcon in its own button that receives keyboard activation;
this means keyboard activation can still hit the default add-tab path and
bypass the extension dropdown logic. Move interception to the actual trigger
button path (or use capture on the wrapper path) so keyboard behavior matches
click behavior. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Keyboard users bypass extension new-tab dropdown behavior.
- ⚠️ SQL Lab tab creation inconsistent between mouse and keyboard.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab so the tab strip renders `TabbedSqlEditors` from
`superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:208-219`
with
`StyledEditableTabs` using `addIcon=<NewTabButton onAddSqlEditor={() =>
newQueryEditor()}
/>` and `onEdit={handleEdit}` at lines 106-118.
2. Inspect `NewTabButton` in the same file at lines 112-204: it renders a
`Tooltip` and
`Dropdown` wrapping a `<span role="button" tabIndex={0} onClick={handleClick}
onKeyDown={handleKeyDown}>` (lines 193-199), with `handleClick` stopping
propagation and
calling `activate()` (lines 167-172) and `handleKeyDown` intercepting
Enter/Space, calling
`preventDefault()`, `stopPropagation()`, and `activate()` (lines 174-182).
3. Note the inline comment at lines 167-176 stating that Ant Design Tabs
wraps `addIcon`
in its own `<button onClick={() => onEdit('add')}>`, meaning the actual
focusable control
and default keyboard activation handlers live on the wrapper button around
`NewTabButton`,
not on the inner span.
4. In the browser, navigate via keyboard to the SQL Lab “Add tab” control
(the AntD add
button that wraps `addIcon`) and press Enter or Space: the keydown and
resulting click
events target the outer button, which invokes `handleEdit('add')` (lines
17-28) and calls
`newQueryEditor()`, while the inner span’s `onKeyDown={handleKeyDown}` never
fires; this
causes keyboard activation to bypass the extension-aware dropdown logic in
`NewTabButton`
even when extensions have contributed `sqllab.newTab` items (declared in
`superset-frontend/src/SqlLab/contributions.ts:40-56`), leading to
inconsistent behaviour
between mouse and keyboard.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9e0e664b257f4ebda0610f85fcefcfee&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=9e0e664b257f4ebda0610f85fcefcfee&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/SqlLab/components/TabbedSqlEditors/index.tsx
**Line:** 174:200
**Comment:**
*Logic Error: The Enter/Space interception is attached to an inner
span, but AntD wraps addIcon in its own button that receives keyboard
activation; this means keyboard activation can still hit the default add-tab
path and bypass the extension dropdown logic. Move interception to the actual
trigger button path (or use capture on the wrapper path) so keyboard behavior
matches click behavior.
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%2F41285&comment_hash=09e5e936a6d35ed9c52619123b8bea64664093e17118dc71ac2d030b478d4eaa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41285&comment_hash=09e5e936a6d35ed9c52619123b8bea64664093e17118dc71ac2d030b478d4eaa&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]
