codeant-ai-for-open-source[bot] commented on code in PR #40563:
URL: https://github.com/apache/superset/pull/40563#discussion_r3329946170
##########
superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx:
##########
@@ -107,13 +108,19 @@ const SqlEditorTabHeader: FC<Props> = ({ queryEditor })
=> {
[dispatch],
);
- function renameTab() {
- // TODO: Replace native prompt with a proper modal dialog
- // eslint-disable-next-line no-alert
- const newTitle = prompt(t('Enter a new title for the tab'));
- if (newTitle) {
- actions.queryEditorSetTitle(qe, newTitle, qe.id);
+ const [showRenameModal, setShowRenameModal] = useState(false);
+ const [newTitle, setNewTitle] = useState('');
+
+ function openRenameModal() {
+ setNewTitle(qe.name);
+ setShowRenameModal(true);
+ }
+
+ function handleRenameConfirm() {
+ if (newTitle.trim()) {
+ actions.queryEditorSetTitle(qe, newTitle.trim(), qe.id);
}
+ setShowRenameModal(false);
}
Review Comment:
**Suggestion:** The confirm handler always closes the modal even when the
input is invalid (empty/whitespace). Because Enter is wired directly to this
handler, users can press Enter with a blank title and the modal closes without
renaming, which bypasses the disabled-state intent of the Rename button. Only
close the modal after a successful rename (or explicitly block Enter when the
trimmed value is empty). [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ SQL Lab tab rename inconsistent for keyboard Enter usage.
- โ ๏ธ Users can dismiss rename modal without explicit cancellation.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open SQL Lab, which renders `TabbedSqlEditors` where the `render()`
method at
`superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:15-24`
maps each
`queryEditor` to a `<SqlEditorTabHeader queryEditor={qe} />` tab label.
2. In the SQL Lab UI, open a tab's context menu, which is implemented by
`MenuDotsDropdown` in `SqlEditorTabHeader` at
`superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx:63-104`,
and click
the `Rename tab` item keyed `'2'` with `onClick: openRenameModal`.
3. Observe that `openRenameModal` at `index.tsx:35-38` sets `newTitle` to
`qe.name` and
`showRenameModal` to `true`, causing the `<Modal>` at `index.tsx:151-180` to
appear with
the `<Input>` bound to `newTitle` and `onPressEnter={handleRenameConfirm}` at
`index.tsx:172-178`.
4. Delete the prefilled tab name so the input is empty (or only whitespace)
and press
Enter: `handleRenameConfirm()` at `index.tsx:40-45` sees `newTitle.trim()`
falsy, skips
`actions.queryEditorSetTitle(...)`, but still executes
`setShowRenameModal(false)`,
closing the modal without renaming despite the `Rename` button being
disabled for the same
invalid state.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e5e28764602941e6bb077ab8c78dddd0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e5e28764602941e6bb077ab8c78dddd0&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/SqlEditorTabHeader/index.tsx
**Line:** 119:124
**Comment:**
*Incorrect Condition Logic: The confirm handler always closes the modal
even when the input is invalid (empty/whitespace). Because Enter is wired
directly to this handler, users can press Enter with a blank title and the
modal closes without renaming, which bypasses the disabled-state intent of the
Rename button. Only close the modal after a successful rename (or explicitly
block Enter when the trimmed value is empty).
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%2F40563&comment_hash=54bf1d0b0ded86ac2fe54224177ab2f41ff6a2a02a455dad6226170d0dd9470c&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40563&comment_hash=54bf1d0b0ded86ac2fe54224177ab2f41ff6a2a02a455dad6226170d0dd9470c&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]
