-
Notifications
You must be signed in to change notification settings - Fork 8
fix: on wasm, track the size of the parent element instead of trying to time explicit layout calls #37
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
base: main
Are you sure you want to change the base?
Conversation
β¦to time explicit layout calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue with Monaco Editor sizing on WASM by replacing manual layout refresh calls with automatic parent element size tracking using ResizeObserver. Instead of relying on explicit LayoutUpdated events and window resize listeners, the editor now observes its parent element directly and adjusts layout automatically when size changes occur.
Key Changes:
- Replaced window resize listener with ResizeObserver for more reliable size tracking
- Removed C# LayoutUpdated event handler that triggered manual layout refreshes
- Removed the
RefreshLayoutJSImport method and its manual invocations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| MonacoEditorComponent/ts-helpermethods/asyncCallbackHelpers.ts | Replaced window resize listener with ResizeObserver to track parent element size changes; removed manual layout calls |
| MonacoEditorComponent/CodeEditor/CodeEditorPresenter.wasm.cs | Removed LayoutUpdated event handler, RefreshLayout JSImport method, and manual layout refresh calls |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resizeObserver.observe(element); | ||
|
|
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ResizeObserver created here is never disconnected, which can lead to a memory leak if the editor element is removed from the DOM. Consider storing the ResizeObserver instance in the EditorContext and disconnecting it when the editor is destroyed or disposed. You should call resizeObserver.disconnect() in a cleanup method.
| resizeObserver.observe(element); | |
| resizeObserver.observe(element); | |
| // Store the observer on the editorContext for later cleanup | |
| editorContext._resizeObserver = resizeObserver; | |
| // Add a dispose method to editorContext to clean up resources | |
| editorContext.dispose = function() { | |
| if (this._resizeObserver) { | |
| this._resizeObserver.disconnect(); | |
| this._resizeObserver = null; | |
| } | |
| if (this.model && typeof this.model.dispose === "function") { | |
| this.model.dispose(); | |
| } | |
| if (this.Accessor && typeof this.Accessor.dispose === "function") { | |
| this.Accessor.dispose(); | |
| } | |
| if (this.Keyboard && typeof this.Keyboard.dispose === "function") { | |
| this.Keyboard.dispose(); | |
| } | |
| if (this.Theme && typeof this.Theme.dispose === "function") { | |
| this.Theme.dispose(); | |
| } | |
| if (this.Debug && typeof this.Debug.dispose === "function") { | |
| this.Debug.dispose(); | |
| } | |
| if (editor && typeof editor.dispose === "function") { | |
| editor.dispose(); | |
| } | |
| }; | |
| // NOTE: You must call editorContext.dispose() when cleaning up the editor to avoid memory leaks. |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GitHub Issue: closes unoplatform/uno#22144
PR Type:
What is the current behavior? π€
What is the new behavior? π
PR Checklist β
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information βΉοΈ