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: [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]
