You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/09/16 20:27:20 UTC
[PR] fix(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
sadpandajoe opened a new pull request, #44359:
URL: https://github.com/apache/superset/pull/44359
### SUMMARY
Fixes two independent dashboard edit-mode bugs reported in #37644.
**Rows not shrinking.** `Row` measured its own `clientHeight` in a `useLayoutEffect` and fed that pixel value back as the explicit inline height of its own vertical droptarget siblings. Since `GridRow` is `height: fit-content`, those pinned siblings then counted toward the very measurement that produced them, so a row's height could grow with its tallest chart but never shrink back down. The droptargets now size purely in CSS — `height: 100%` for the absolutely-positioned ones, `align-self: stretch` for the in-flow ones — and the self-referential effect and its `containerHeight` state are gone.
**Charts above a column could not be resized.** A column's `HoverMenu` is absolutely positioned above the column, so it necessarily overlaps the bottom edge of whatever sits above it. Its `z-index: 11` beat the sibling resize handles' `z-index: 10`, so hovering the bottom edge or bottom-right corner of a chart directly above a column hit the column's menu rather than the chart's handle, and no resize cursor appeared. Resize handles now stack one level above the hover menu.
Rather than leaving three interdependent magic numbers scattered across files, the stacking levels this bug turns on are named in `src/dashboard/constants.ts` and derived from each other, so the required ordering (`empty droptarget < hover menu < resize handle`) is stated once. The handle deliberately sits at 12 rather than a large value: the surrounding code documents a scale where 99 is dashboard popovers and 100 the dashboard header, and a chart-local handle — which still hit-tests at `opacity: 0` — should not reach that layer.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
### TESTING INSTRUCTIONS
Row shrinking:
1. Open a dashboard in edit mode and put two charts in the same row.
2. Increase one chart's height — the row grows.
3. Decrease the heights of all charts in the row — the row now shrinks to fit (previously it stayed at its high-water mark).
Resizing a chart above a column:
1. In edit mode, add two charts to one row.
2. Add a column layout element to the row below, and a chart to the row below that.
3. Hover the bottom edge and bottom-right corner of the chart in the first row that sits directly above the column — the resize cursor appears and vertical/diagonal resize works.
4. Confirm the column's own hover menu (drag handle, delete, settings) is still reachable and that its buttons still work.
Automated: `npm run test -- src/dashboard/components/gridComponents/Row src/dashboard/components/menu src/dashboard/components/resizable`. `Row.test.tsx` contains a regression test that reproduces the measure-and-pin feedback loop and fails without this change.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #37644
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5810685493
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 an incremental AI Review.
- `/review full` - 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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5828950282
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 an incremental AI Review.
- `/review full` - 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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5704207057
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44359?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 80.28%. Comparing base ([`637e19f`](https://app.codecov.io/gh/apache/superset/commit/637e19f0ca384e111b55e5727e07697d39f49c1c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`faed628`](https://app.codecov.io/gh/apache/superset/commit/faed6288a06d3bd96fbb61c1de717d73b08b43c9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44359 +/- ##
==========================================
- Coverage 80.28% 80.28% -0.01%
==========================================
Files 2929 2929
Lines 174153 174148 -5
Branches 40416 40413 -3
==========================================
- Hits 139822 139817 -5
Misses 31674 31674
Partials 2657 2657
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44359/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/44359/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.69% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44359?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5705142525
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 5843787c0933cf5c3077cac4460e4a290bbfc7d4 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aab100fd3461600087bef8c |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44359--superset-docs-preview.netlify.app](https://deploy-preview-44359--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#q1YqKMrPLShRslJS0lFKKkrMS84AsktzSooSi1NLdNMyK3SNzc1MTICyKakFOfmVnilAebPExCRDA4O0FGMTM0MzAwMDC_Ok1DSLZKVaAA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44359--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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4092124955
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx:
##########
@@ -269,6 +278,25 @@ test('should increment the depth of its children', () => {
);
});
+test('row droptargets size via CSS instead of a measured pixel height that can only grow (regression for #37644)', () => {
+ const { container } = setup({ editMode: true });
+ const getDroptargetHeights = () =>
+ Array.from(
+ container.querySelectorAll<HTMLElement>('.empty-droptarget--vertical'),
+ ).map(el => el.style.height);
+
+ // The leading droptarget (index 0) is absolutely positioned, so a
+ // percentage height resolves fine and stretches it to the row via CSS
+ // rather than a pixel value measured from the row's tallest chart -- it
+ // can never be left pinned to a chart's prior (larger) height after a
+ // resize. The droptarget after the chart is an in-flow flex item under
+ // GridRow's indefinite `height: fit-content`, where a percentage height
+ // resolves to `auto`/is ignored per the flexbox spec (defeating the
+ // `align-self: stretch` CSS already declares for it) -- it gets an
+ // explicit `auto` instead, letting that stretch actually apply.
+ expect(getDroptargetHeights()).toEqual(['100%', 'auto']);
Review Comment:
Nit, take it or leave it. The fixture row is half full (6 of 12 columns), so `isTrailingSideTarget` is never true here and the branch the last commit refactored goes untested.
A second case with `occupiedColumnCount: 12` expecting `['100%', '100%']` would pin the side target as well.
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5789194076
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 an incremental AI Review.
- `/review full` - 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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5704037536
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #44359:
URL: https://github.com/apache/superset/pull/44359
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [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 #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4078279942
##########
superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx:
##########
@@ -130,7 +131,7 @@ const StyledResizable = styled(Resizable)`
.resize-handle {
opacity: 0;
- z-index: 10;
+ z-index: ${RESIZE_HANDLE_Z_INDEX};
Review Comment:
**Suggestion:** Column drag styles use a more specific selector with `z-index: 10`, so this `z-index: 12` is overridden and column resize handles still lose hit testing to hover menus.
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Css layout issue`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e729998b05f34b5180c7cd006f4b7126&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=e729998b05f34b5180c7cd006f4b7126&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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/resizable/ResizableContainer.tsx
**Line:** 134:134
**Comment:**
*Css Layout Issue: Column drag styles use a more specific selector with `z-index: 10`, so this `z-index: 12` is overridden and column resize handles still lose hit testing to hover menus.
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%2F44359&comment_hash=3bb29f555f4a26d730a14e768196229db682b1e0d6431b20bdc46d03e06bf43b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44359&comment_hash=3bb29f555f4a26d730a14e768196229db682b1e0d6431b20bdc46d03e06bf43b&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx:
##########
@@ -403,7 +394,7 @@ const Row = memo((props: RowProps) => {
)}
editMode
style={{
- height: rowItems.length > 0 ? containerHeight : '100%',
+ height: '100%',
Review Comment:
**Suggestion:** In an empty row, the absolute droptarget's parent has only `height: fit-content`; its `100%` height resolves as `auto`, so the target can collapse and lose the row's drop area.
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Css layout issue`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bc520ac0a5e1401f9af5a84023b9f330&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=bc520ac0a5e1401f9af5a84023b9f330&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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/gridComponents/Row/Row.tsx
**Line:** 397:397
**Comment:**
*Css Layout Issue: In an empty row, the absolute droptarget's parent has only `height: fit-content`; its `100%` height resolves as `auto`, so the target can collapse and lose the row's drop area.
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%2F44359&comment_hash=1c484d3e2d5e3e391612835c7a80496a1346fa94e445e117e8e43eec97539031&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44359&comment_hash=1c484d3e2d5e3e391612835c7a80496a1346fa94e445e117e8e43eec97539031&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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4030593871
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx:
##########
@@ -269,6 +278,57 @@ test('should increment the depth of its children', () => {
);
});
+test('row droptarget height should shrink after the tallest chart in the row is resized smaller (regression for #37644)', () => {
+ // Emulates a flex row sized to fit-content: its clientHeight is at least
+ // as tall as its tallest child, including a droptarget sibling whose
+ // height was explicitly set to a pixel value by a prior render. This
+ // mirrors how a real browser lays out GridRow and its droptargets.
+ const clientHeightDescriptor = Object.getOwnPropertyDescriptor(
+ Element.prototype,
+ 'clientHeight',
+ );
+ let trueContentHeight = 300;
+
+ Object.defineProperty(Element.prototype, 'clientHeight', {
+ configurable: true,
+ get(this: Element) {
+ if (!this.classList.contains('grid-row')) return 0;
+ const appliedHeights = Array.from(
+ this.querySelectorAll<HTMLElement>('.empty-droptarget--vertical'),
+ ).map(el => parseFloat(el.style.height) || 0);
+ return Math.max(trueContentHeight, 0, ...appliedHeights);
+ },
+ });
+
+ try {
+ const { container, rerender } = setup({ editMode: true });
+ const getDroptargetHeight = () =>
+ container.querySelector<HTMLElement>('.empty-droptarget--vertical')?.style
+ .height;
+
+ // Sanity: the droptarget renders a height while the row's tallest
+ // chart is large.
+ expect(getDroptargetHeight()).toBeTruthy();
+
+ // The tallest chart in the row shrinks, then something (e.g. hovering
+ // the row's own HoverMenu while resizing) causes Row to re-render.
+ trueContentHeight = 100;
+ rerender(<Row {...props} editMode component={{ ...props.component }} />);
+
+ // The droptarget must track the shrunk content instead of staying
+ // pinned to the chart's prior (larger) measured height.
+ expect(getDroptargetHeight()).not.toBe('300px');
+ } finally {
+ if (clientHeightDescriptor) {
+ Object.defineProperty(
+ Element.prototype,
+ 'clientHeight',
+ clientHeightDescriptor,
+ );
+ }
+ }
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inert test mock</b></div>
<div id="fix">
This test mocks `Element.prototype.clientHeight`, but this PR removed the `containerHeight` state and `useLayoutEffect` that read `clientHeight` from `Row.tsx`; the droptarget height is now hardcoded to `'100%'`. The mock is inert, so `toBeTruthy()` and `not.toBe('300px')` pass trivially regardless of the regression fix, giving false confidence. Assert the real behavior (height stays `'100%'`) or remove the mock.
</div>
</div>
<small><i>Code Review Run #ac02b6</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5769835513
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 an incremental AI Review.
- `/review full` - 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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4079154688
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx:
##########
@@ -450,7 +441,22 @@ const Row = memo((props: RowProps) => {
)}
editMode
style={{
- height: containerHeight,
+ // Only the last target in a full row (the absolutely
+ // positioned "side" target) actually resolves a
+ // percentage height -- for every other, in-flow
+ // target, GridRow's height is `fit-content`
+ // (indefinite), and a percentage height on a flex
+ // item under an indefinite-height parent resolves to
+ // `auto`/is ignored per the CSS flexbox spec, which
+ // defeats the `align-self: stretch` the CSS above
+ // already declares for it. `height: 'auto'` lets that
+ // stretch actually apply instead of collapsing the
+ // target to its content size.
+ height:
+ remainColumnCount === 0 &&
+ itemIndex === rowItems.length - 1
+ ? '100%'
+ : 'auto',
...(remainColumnCount === 0 &&
itemIndex === rowItems.length - 1 && { width: 16 }),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated side-target condition</b></div>
<div id="fix">
The predicate `remainColumnCount === 0 && itemIndex === rowItems.length - 1` is duplicated for the `height` ternary and the `width` spread. If one is edited without the other, the side target's height and width diverge. Extract it to a local const in the map callback.
</div>
</div>
<small><i>Code Review Run #ad471a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4066187742
##########
superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx:
##########
@@ -130,7 +131,7 @@ const StyledResizable = styled(Resizable)`
.resize-handle {
opacity: 0;
- z-index: 10;
+ z-index: ${RESIZE_HANDLE_Z_INDEX};
Review Comment:
Could we also address the actual resize hit areas and their ancestor stacking contexts here? `.resize-handle` is the small visual grip inside re-resizable's event-handling element; raising it doesn't raise the full bottom/corner target. Also, charts in a row and the column below both get `.dragdroppable-column`, whose `transform: translate3d(0, 0, 0)` creates separate stacking contexts. The chart's descendants can't escape that context just by increasing their z-index.
In a small Chromium reproduction using these styles and re-resizable, `elementFromPoint` at the chart's bottom edge still returned the column's `.hover-menu` with this change, so the reported resize problem remains. Raising only the event-handling element also left it blocked; the ancestor stacking order needs attention too. A browser check of the bottom edge and corner, alongside the column menu buttons, would help verify the fix.
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx:
##########
@@ -450,7 +441,7 @@ const Row = memo((props: RowProps) => {
)}
editMode
style={{
- height: containerHeight,
+ height: '100%',
Review Comment:
Could we use `height: 'auto'` for the in-flow drop targets and keep `100%` for the absolutely positioned side targets? The percentage height here prevents `align-self: stretch` from filling the row. In a small Chromium reproduction using the `GridRow` and `GridContent` styles, a 300px row had a 300px side target but only 2px-high targets between/after the charts. After shrinking the row to 200px, those targets were still 2px high, which makes the gaps effectively unavailable for dropping charts. Setting the in-flow targets to `auto` made them fill the 200px row.
The new test only selects the first, absolutely positioned target, so it misses this case. It would help to cover the rendered height of a between-chart target too.
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5768203000
Pushed a fix for @EnxDev's first finding. The in-flow drop targets were collapsing because a percentage height on a flex item under GridRow's indefinite height resolves to `auto` per the flexbox spec, defeating the `align-self: stretch` this PR already declares. `height: 'auto'` fixes it, mirroring the width condition already used a few lines down for the trailing side target, and the regression test now checks both targets instead of just the first.
The resize-handle z-index finding is the harder one though. `.resize-handle` isn't the actual hit-testing element, and `.dragdroppable-column`'s `translate3d` creates a stacking context the z-index can't escape. That needs real rework (dropping/reworking the transform, or portal-rendering the handles the way `ResizableContainer` already does for `CursorLabel`), not a quick patch. @EnxDev, curious your take on the right approach there before this merges.
--
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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44359:
URL: https://github.com/apache/superset/pull/44359#issuecomment-5706776207
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 an incremental AI Review.
- `/review full` - 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(dashboard): shrink row height on chart resize and unblock resize handles above columns [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44359:
URL: https://github.com/apache/superset/pull/44359#discussion_r4087731785
##########
superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx:
##########
@@ -269,6 +278,31 @@ test('should increment the depth of its children', () => {
);
});
+test('row droptarget height tracks the row instead of staying pinned to a stale measured height (regression for #37644)', () => {
+ const { container, rerender } = setup({ editMode: true });
+ const getDroptargetHeights = () =>
+ Array.from(
+ container.querySelectorAll<HTMLElement>('.empty-droptarget--vertical'),
+ ).map(el => el.style.height);
+
+ // The leading droptarget (index 0) is absolutely positioned, so a
+ // percentage height resolves fine and stretches it to the row via CSS
+ // rather than a pixel value measured from the row's tallest chart -- it
+ // can never be left pinned to a chart's prior (larger) height after a
+ // resize. The droptarget after the chart is an in-flow flex item under
+ // GridRow's indefinite `height: fit-content`, where a percentage height
+ // resolves to `auto`/is ignored per the flexbox spec (defeating the
+ // `align-self: stretch` CSS already declares for it) -- it gets an
+ // explicit `auto` instead, letting that stretch actually apply.
+ expect(getDroptargetHeights()).toEqual(['100%', 'auto']);
+
+ // Something (e.g. hovering the row's own HoverMenu while resizing)
+ // causes Row to re-render after the tallest chart in the row shrinks.
+ rerender(<Row {...props} editMode component={{ ...props.component }} />);
+
+ expect(getDroptargetHeights()).toEqual(['100%', 'auto']);
Review Comment:
The rerender passes the same props and jsdom has no layout, so nothing shrinks here and this expect can't fail unless the one above already did.
The test also pins inline style strings, so it stays green if `align-self: stretch` comes out of `GridRow`, and that's the line doing the real work. I'd drop the rerender half and the "reproduces the feedback loop" line in the description. A real guard for the layout needs a browser, e.g. a Playwright check on the droptarget height after shrinking a chart.
##########
superset-frontend/src/dashboard/components/menu/HoverMenu.tsx:
##########
@@ -32,7 +34,7 @@ const HoverStyleOverrides = styled.div`
.hover-menu {
opacity: 0;
position: absolute;
- z-index: 11; /* one more than DragDroppable */
+ z-index: ${HOVER_MENU_Z_INDEX};
Review Comment:
For the resize part I'd fix it here rather than on the handles: this menu still hit-tests at `opacity: 0`, and that invisible box is what covers the bottom edge of the chart above.
`pointer-events: none` here, plus `pointer-events: auto` next to the `opacity: 1` reveal in `DashboardWrapper.tsx:98`, lets the edge through without touching the `translate3d` stacking contexts. The catch is that a column's menu only becomes clickable once the pointer has entered the column, so worth checking those buttons in a browser.
##########
superset-frontend/src/dashboard/constants.ts:
##########
@@ -43,6 +43,12 @@ export const FILTER_BAR_TABS_HEIGHT = 46;
export const BUILDER_SIDEPANEL_WIDTH = 374;
export const OVERWRITE_INSPECT_FIELDS = ['css', 'json_metadata.filter_scopes'];
export const EMPTY_CONTAINER_Z_INDEX = 10;
+// A grid component's hover menu floats outside the component's own box, so it
+// must paint above the drop targets it overlaps.
+export const HOVER_MENU_Z_INDEX = EMPTY_CONTAINER_Z_INDEX + 1;
+// A hover menu rendered above a column overlaps the bottom edge of whatever
+// sits above it, so resize handles have to win hit testing against it.
+export const RESIZE_HANDLE_Z_INDEX = HOVER_MENU_Z_INDEX + 1;
Review Comment:
Per the thread on `ResizableContainer.tsx`, this comment promises something z-index can't deliver, since the chart and the column below it sit in separate `translate3d` stacking contexts.
If the pointer-events change on the menu pans out, I'd drop `RESIZE_HANDLE_Z_INDEX` (or reword this) so the constants don't document an ordering that doesn't hold.
--
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