codeant-ai-for-open-source[bot] commented on code in PR #38745:
URL: https://github.com/apache/superset/pull/38745#discussion_r2960850087
##########
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:
##########
@@ -626,6 +626,9 @@ const PropertiesModal = ({
form.submit();
}
}}
+ resizable
+ draggable
Review Comment:
**Suggestion:** Enabling both drag/resize on this modal changes core modal
behavior to remove the backdrop mask, so users can interact with the dashboard
behind an open edit form and trigger conflicting state changes while unsaved
properties are being edited. Keep this modal non-draggable/non-resizable to
preserve modal isolation. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard header actions clickable during open properties modal.
- ⚠️ Unsaved properties can conflict with background dashboard edits.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enter dashboard edit mode, then open the header actions menu and click
**Edit
properties**; this executes `showPropertiesModal()` in
`superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:93-95`.
2. `showPropertiesModal` sets state in
`superset-frontend/src/dashboard/components/Header/index.tsx:512-514`, then
`PropertiesModal` is rendered at `Header/index.tsx:844-850`.
3. The modal is created with `resizable` and `draggable` in
`superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:629-631`.
4. Core modal logic computes `const shouldShowMask = !(resizable ||
draggable)` at
`superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx:280`
and
applies `mask={shouldShowMask}` at `Modal.tsx:360`, so mask becomes `false`.
5. While the properties modal stays open, click background dashboard
controls like
`Discard`/`Save`
(`superset-frontend/src/dashboard/components/Header/index.tsx:724-737`);
these remain clickable because no mask blocks background interaction.
6. Observe conflicting edit flow: background dashboard actions run while
properties form
is still unsaved/open, breaking modal isolation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
**Line:** 629:630
**Comment:**
*Logic Error: Enabling both drag/resize on this modal changes core
modal behavior to remove the backdrop mask, so users can interact with the
dashboard behind an open edit form and trigger conflicting state changes while
unsaved properties are being edited. Keep this modal
non-draggable/non-resizable to preserve modal isolation.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38745&comment_hash=0a1fc519d074235828cba2511e420f47cfc4114adddf708b7dd2a148c047051d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38745&comment_hash=0a1fc519d074235828cba2511e420f47cfc4114adddf708b7dd2a148c047051d&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Modal/StandardModal.tsx:
##########
@@ -50,10 +52,9 @@ export const MODAL_LARGE_WIDTH = 900;
const StyledModal = styled(Modal)`
.ant-modal-body {
- max-height: 60vh;
+ max-height: 80vh;
Review Comment:
**Suggestion:** The fixed `max-height: 80vh` on the modal body breaks the
new resizable behavior: once users resize past that height, the body stops
growing and leaves unusable empty space, so large JSON editors still cannot
fully expand. Make the max-height conditional so it is only applied when the
modal is not resizable. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ⚠️ Dashboard properties modal wastes space when resized taller.
- ⚠️ JSON metadata editor expansion is capped unexpectedly.
```
</details>
```suggestion
max-height: ${({ resizable }) => (resizable ? 'none' : '80vh')};
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a dashboard in edit mode and click "Edit properties"; this action is
wired in
`superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:93-95`
(`MenuKeys.EditProperties -> showPropertiesModal()`), which toggles modal
state in
`superset-frontend/src/dashboard/components/Header/index.tsx:512-513`.
2. Confirm the modal render path: `Header/index.tsx:844-850` conditionally
renders
`<PropertiesModal ... show={showingPropertiesModal} />`, and
`superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:621-631`
passes
`resizable` and `draggable` to `<StandardModal>`.
3. In `superset-frontend/src/components/Modal/StandardModal.tsx:54-56`,
`.ant-modal-body`
is capped with `max-height: 80vh`; meanwhile core resizable logic sets body
fill-height in
`superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx:175-179`.
4. Drag-resize the properties modal vertically beyond ~80vh; the body stops
growing
(capped), leaving unused space while advanced JSON editor container
(`superset-frontend/src/dashboard/components/PropertiesModal/sections/AdvancedSection.tsx:78-89`)
cannot fully expand with modal height.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/Modal/StandardModal.tsx
**Line:** 55:55
**Comment:**
*Logic Error: The fixed `max-height: 80vh` on the modal body breaks the
new resizable behavior: once users resize past that height, the body stops
growing and leaves unusable empty space, so large JSON editors still cannot
fully expand. Make the max-height conditional so it is only applied when the
modal is not resizable.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38745&comment_hash=a4626938b6a9568d0c3f3262f082ccdcfa18f7a10dc1fd8167abc4cfc1a3b298&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38745&comment_hash=a4626938b6a9568d0c3f3262f082ccdcfa18f7a10dc1fd8167abc4cfc1a3b298&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]
