You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "RoySerbi (via GitHub)" <gi...@apache.org> on 2026/05/31 08:39:15 UTC
[PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
RoySerbi opened a new pull request, #40563:
URL: https://github.com/apache/superset/pull/40563
## What this does
Fixes #40401
Replaces the native browser `prompt()` dialog with a styled Superset Modal for the SQL Lab tab rename feature.
**Before:** Native browser `prompt()` dialog (unstyled, blocked in some environments)
**After:** Superset-styled Modal with Input field, consistent with the rest of the UI
## Changes
- `SqlEditorTabHeader/index.tsx`: Replaced `prompt()` call with a controlled `Modal` component containing an `Input` field
- Added `useState` for modal visibility and title state
- Modal pre-fills the current tab name
- Rename button disabled when input is empty
- Supports Enter key to confirm
- Follows the same Modal pattern used by `CREATE TABLE/VIEW AS` in `SqlEditor/index.tsx`
## How it works
Clicking "Rename tab" in the context menu now opens a styled Modal dialog instead of the browser's native prompt. The modal includes:
- A text input pre-filled with the current tab name
- Cancel and Rename buttons (Rename is disabled when input is empty)
- Enter key support for quick confirmation
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4816022348
Ahh, looks like #41329 landed the modal version of this already and closed the issue out. I think we close this one. Thanks for taking a swing at it @RoySerbi, and we'll get better about keeping up with PR reviews and such so things like this don't get stuck in the future.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #40563: fix: replace native prompt() with styled modal for SQL Lab tab rename
URL: https://github.com/apache/superset/pull/40563
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4586196424
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "RoySerbi (via GitHub)" <gi...@apache.org>.
RoySerbi commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4599311796
๏ปฟ@aminghadersohi thanks for the thorough review. `1a7e4dd` addresses each item:
**BLOCKER โ rename test rewritten**
The `should dispatch queryEditorSetTitle action` test no longer stubs `window.prompt`. It now drives the full modal flow: clicks `rename-tab-menu-option`, waits for `rename-tab-input` to appear, fires `change` with the desired title, clicks `rename-tab-confirm`, and asserts `QUERY_EDITOR_SET_TITLE` is dispatched. Renamed to `should dispatch queryEditorSetTitle action via modal confirm`.
Also added the two follow-up cases you suggested:
- `should dispatch queryEditorSetTitle action when pressing Enter` - fires a keydown Enter on the input and verifies the same dispatch (covers onPressEnter).
- `cancel button closes modal without dispatching` - types text, clicks Cancel, asserts the modal unmounts and no QUERY_EDITOR_SET_TITLE was dispatched.
Plus an extra:
- `confirm with empty input does not dispatch and keeps modal open` - covers the codeant-bot edge case where pressing Enter on empty/whitespace input must not silently close the modal. The existing early-return at `index.tsx:120` already handles this; the new test pins the behaviour.
The Cancel test required exposing `data-test="rename-tab-cancel"` on the Cancel button - included in this commit.
**MEDIUM - autoFocus added**
Added `autoFocus` to the rename `<Input>`. The input is now focused as soon as the modal opens.
**NIT - trim on change**
`onChange` now sets `e.target.value.trim()`, so the displayed value matches what would be saved. (Quick note: String.trim() only strips leading/trailing whitespace, so internal spaces are preserved. One edge case is typing a trailing space and then more text strips the space; happy to switch to a leading-only strip + onBlur trim if you prefer.)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4775639413
@RoySerbi right direction, but `sharded-jest-tests (7)` is red on this PR's own suite โ `cancel button closes modal without dispatching` fails at `SqlEditorTabHeader.test.tsx:209` because the input is still in the DOM after Cancel.
Also `index.tsx`: the `Input` `onChange` does `e.target.value.trim()`, so spaces get stripped while typing and you can't enter a title like `my query`. Trim on confirm, not on change.
Happy to help touch things up if you want help on this one (assuming I can push to the branch)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4586891928
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4599329012
### <span aria-hidden="true">โ
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">๐จ</span> Latest commit | 1a7e4dd07623c209ce2f023664f541ab295610ed |
|<span aria-hidden="true">๐</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a1e76b6b9c19100088157bb |
|<span aria-hidden="true">๐</span> Deploy Preview | [https://deploy-preview-40563--superset-docs-preview.netlify.app](https://deploy-preview-40563--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">๐ฑ</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">๐ค</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#Dco5DoAgEADAv2wtkS3k8Ac-YxcwmnCJFhrj36WbYl6oraR6wQwwADfKbute93s8jygisWghUwoiFU-xHx9qLM_i-1KEQStWbB1alFIag5Nmhu8H) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40563--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40563:
URL: https://github.com/apache/superset/pull/40563#discussion_r3417230487
##########
superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx:
##########
@@ -220,6 +228,40 @@ const SqlEditorTabHeader: FC<Props> = ({ queryEditor }) => {
iconSize="m"
iconColor={getStatusColor(queryState, theme)}
/>{' '}
+ <Modal
Review Comment:
**CI failure โ `sharded-jest-tests (7)`**
The "cancel button closes modal without dispatching" test is currently failing in CI because `@superset-ui/core/components/Modal` keeps its children mounted in the DOM when closed (Ant Design default). After clicking Cancel, the `rename-tab-input` is still present in the DOM (`value="discarded"`), so `expect(...).not.toBeInTheDocument()` fails.
Fix: add `destroyOnHidden` so the modal fully unmounts its children on close:
```suggestion
<Modal
destroyOnHidden
show={showRenameModal}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4586207670
<!-- Bito Reply -->
The provided PR comments file is empty or contains only header information without any actual comments. I cannot analyze or validate the suggestion without the necessary content.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: replace native prompt() with styled modal for SQL Lab tab rename [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40563:
URL: https://github.com/apache/superset/pull/40563#issuecomment-4601021201
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org