codeant-ai-for-open-source[bot] commented on code in PR #42548:
URL: https://github.com/apache/superset/pull/42548#discussion_r3671818678
##########
superset-frontend/packages/superset-ui-core/src/components/UnsavedChangesModal/UnsavedChangesModal.test.tsx:
##########
@@ -94,3 +95,39 @@ test('should only call handleSave when clicking the Save
button', async () => {
expect(mockOnHide).not.toHaveBeenCalled();
expect(mockOnConfirmNavigation).not.toHaveBeenCalled();
});
+
+test('renders above an already-open modal without a hardcoded z-index', () => {
+ // Regression test for a bug where this modal could render BEHIND another
+ // already-open modal (e.g. a draggable "View query" modal), because its
+ // z-index was pinned to a hardcoded constant instead of relying on Ant
+ // Design's automatic z-index stacking. Since this modal is always opened
+ // on top of whatever it's interrupting, it should always come out ahead
+ // with no manual override at all.
+ render(
+ <>
+ <Modal show title="Other open modal" onHide={() => {}}>
+ <div>Other modal content</div>
+ </Modal>
+ <UnsavedChangesModal
+ showModal
+ onHide={() => {}}
+ handleSave={() => {}}
+ onConfirmNavigation={() => {}}
+ />
+ </>,
+ );
Review Comment:
**Suggestion:** The regression test mounts both modals with `show` already
true in a single render, but production keeps `UnsavedChangesModal` mounted
while `showModal` is false and opens it later. This does not exercise the open
transition or portal insertion order that determines Ant Design's stacking
behavior, so the test can pass while the real navigation flow still renders the
unsaved-changes modal behind an existing modal. Render the unsaved modal hidden
first, then rerender it with `showModal` enabled after the other modal is open.
[possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Regression test misses production open-transition ordering.
- ⚠️ Dashboard navigation modal stacking remains insufficiently verified.
- ⚠️ Explore navigation uses the same mounted-modal pattern.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The production dashboard header always mounts `UnsavedChangesModal` at
`superset-frontend/src/dashboard/components/Header/index.tsx:915-921`,
passing the
hook-controlled `showUnsavedChangesModal` value rather than conditionally
mounting the
component.
2. The navigation blocker in
`superset-frontend/src/hooks/useUnsavedChangesPrompt/index.ts:96-106`
changes that value
from false to true after an existing modal is already open, so the
unsaved-changes modal
is inserted or opened during a later render.
3. The regression test at
`superset-frontend/packages/superset-ui-core/src/components/UnsavedChangesModal/UnsavedChangesModal.test.tsx:106-118`
instead renders both `Modal` instances with `show`/`showModal` already true
in the same
render, which does not model the production transition.
4. Because Ant Design computes automatic modal stacking when dialogs open,
this test can
observe the expected ordering for initial render while missing a regression
in the later
open transition; initialize `UnsavedChangesModal` with `showModal={false}`,
assert the
other dialog is open, then call `rerender` with `showModal` enabled before
comparing
z-indexes.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2024545ba73243cb88b85b0cc68932fd&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=2024545ba73243cb88b85b0cc68932fd&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/packages/superset-ui-core/src/components/UnsavedChangesModal/UnsavedChangesModal.test.tsx
**Line:** 106:118
**Comment:**
*Possible Bug: The regression test mounts both modals with `show`
already true in a single render, but production keeps `UnsavedChangesModal`
mounted while `showModal` is false and opens it later. This does not exercise
the open transition or portal insertion order that determines Ant Design's
stacking behavior, so the test can pass while the real navigation flow still
renders the unsaved-changes modal behind an existing modal. Render the unsaved
modal hidden first, then rerender it with `showModal` enabled after the other
modal is open.
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%2F42548&comment_hash=49bdc477b01677d1d6c97c61e8dfe45cfd6be5ecd07e272cf1c53617b12ff313&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42548&comment_hash=49bdc477b01677d1d6c97c61e8dfe45cfd6be5ecd07e272cf1c53617b12ff313&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]
