Fix modal popup layout and scrolling #6413
Conversation
|
E2E Tests 🚀 |
...rerPanel/components/addEditRowFilterModalPopup/components/columnSelectorDataGridInstance.tsx
Outdated
Show resolved
Hide resolved
src/vs/workbench/browser/positronComponents/positronModalPopup/positronModalPopup.tsx
Outdated
Show resolved
Hide resolved
...aExplorerPanel/components/addEditRowFilterModalPopup/components/columnSelectorModalPopup.tsx
Outdated
Show resolved
Hide resolved
...rerPanel/components/addEditRowFilterModalPopup/components/columnSelectorDataGridInstance.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
This changed worked for the console info panel! There was some scenarios for small widths that caused the popup to get cut off but I think that has to do with the anchor point for the popup getting cut off and not scaling. Overall a much better improvement than what we had before.
I did have a comment and a couple questions but overall looks great!
| positionBottom(); | ||
| } else if (childrenHeight < anchorLayout.anchorY - 1) { | ||
| } if (props.popupPosition === 'top') { |
There was a problem hiding this comment.
Should this be an else if since its nested as part of this entire if/else block?
| // Calculate the max height. This is the height of the search UI plus the height of the rows, | ||
| // plus the height of the top and bottom rows margin. | ||
| const maxHeight = | ||
| (enableSearch ? 34 : 0) + |
There was a problem hiding this comment.
Should we make 34 a constant value called SEARCH_BAR_HEIGHT or something like that so it feels less like a magic number?
| }; | ||
|
|
||
| /** | ||
| * Positions the popup aligned with the right edge of the anchor element. | ||
| */ | ||
| const positionRight = () => { | ||
| popupLayout.right = documentWidth - (anchorLayout.anchorX + anchorLayout.anchorWidth); | ||
| if (isNumber(props.width)) { | ||
| popupLayout.left = (anchorX + anchorWidth) - props.width - LAYOUT_OFFSET; |
There was a problem hiding this comment.
This is probably just a me thing, but what is LAYOUT_OFFSET representing? Is this taking into account a border? or is it just shifting the layout over a tad bit?
There was a problem hiding this comment.
This was a bug. - LAYOUT_OFFSET has been removed.
This PR dealt almost exclusively with vertical layout and vertical scrolling, which is where the major problems of #3061 were to be found. I still have to deal with horizontal layout issues. I was not able to do this work in this PR because it was time-boxed for this milestone. |
Description
This PR addresses #3061 by updating how modal popups are laid out.
Tests:
@:data-explorer
@:new-project-wizard
Release Notes
New Features
Bug Fixes
QA Notes
We will need to think about how we can test this code in an end-to-end test.