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

[Feature Request] Use resizeObserver for editor layout. #1266

Closed
ghost opened this issue Jan 10, 2019 · 7 comments · Fixed by microsoft/vscode#90111
Closed

[Feature Request] Use resizeObserver for editor layout. #1266

ghost opened this issue Jan 10, 2019 · 7 comments · Fixed by microsoft/vscode#90111
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@ghost
Copy link

ghost commented Jan 10, 2019

The way of checking element width every 100ms is not ideal. Use resizeObserver instead.
https://developers.google.com/web/updates/2016/10/resizeobserver

@alexdima alexdima self-assigned this Jan 10, 2019
@alexdima alexdima added this to the On Deck milestone Jan 10, 2019
@rebornix
Copy link
Member

rebornix commented Jan 23, 2019

We can probably user MutationObserver for IE and ResizeObserver for browsers support that.

@dcecile
Copy link

dcecile commented Dec 4, 2019

Instead of using automaticLayout with its "severe performance impact", my current workaround looks like this:

div.page (display flex column)
    div.content-before
    div.wrapper (flex grow, position relative)
        div.container (position absolute, width 100%, height 100%)
            monaco
    div.content-after
  • The wrapper div fills the available space on the page page (space not used by content before or after)
  • Because the container has position absolute, Monaco won't be able to "lock in" extra space on the page during page shrinking
  • When the wrapper div resizes (I'm using react-resize-detector), I call editor.layout()

This works for both growing and shrinking the page. Monaco quickly resizes to the correct size.

@ghost
Copy link
Author

ghost commented Dec 4, 2019

@dcecile This is not related to your use case. That lib uses resize-observer-polyfill which uses mutation observer. We don't need to fake or polyfill. And this request asks for an implementation change.

@alexdima alexdima added the feature-request Request for new features or functionality label Dec 12, 2019
@alexdima alexdima modified the milestones: On Deck, Backlog Dec 16, 2019
@nrayburn-tech
Copy link
Contributor

@alexdima I think that I can implement something for this, but wanted to run my ideas by somebody before working on it.

Modify the ElementSizeObserver to create a MutationObserver on the editor element. When the observer is triggered, we will update the editor size. It is my understanding that MutationObserver only catches changes to the DOM, so we would need to bind another listener to the window resize event.

As mentioned above, a ResizeObserver can be used instead for the browsers that support it.

Does that sound correct? If so, I can work on creating a PR for it.

@alexdima
Copy link
Member

alexdima commented Feb 5, 2020

@nrayburn-tech 👍 PR welcome

@nrayburn-tech
Copy link
Contributor

@alexdima Should this also be used in the diffEditorWidget?

@alexdima
Copy link
Member

@nrayburn-tech Good point. I have pushed microsoft/vscode@88146d9 for this.

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants