Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fix/occluding viewer panes #2608

Merged
merged 3 commits into from
Apr 3, 2024
Merged

Conversation

nstrayer
Copy link
Contributor

@nstrayer nstrayer commented Apr 2, 2024

Intent

Fix problem where viewer and help-pane can occlude other parts of the editor.

Addresses #1997

Approach

This issue was caused by there being a minimum height for the vertical size of a ViewPane of 120 pixels.

This was causing the layout method to never tell the editor to be smaller than 120px tall.

Copy link
Collaborator

@jmcphers jmcphers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the observed size when this happens? I wonder if another way to do this would be to disown/detach the overlay webview when the size is sufficiently small.

Totally fine with this workaround for now, though.

@nstrayer
Copy link
Contributor Author

nstrayer commented Apr 3, 2024

There are cases where it would look weird if the webview disappeared at smaller than 120px sizes. I'll add a note though about potentially detaching when the size gets to the point where nothing is visible, although maybe the cost of disposing and then reopening could be greater than just leaving a short webview around? I'm not familiar with the overhead, but we wont ever have an unbounded number sitting there.

@nstrayer nstrayer merged commit c329257 into main Apr 3, 2024
1 check passed
@nstrayer nstrayer deleted the bug-fix/occluding-viewer-panes branch April 3, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants