Skip to content
This repository has been archived by the owner on Mar 22, 2024. It is now read-only.

Improve configuration change detection #47

Merged
merged 2 commits into from
Sep 20, 2023
Merged

Improve configuration change detection #47

merged 2 commits into from
Sep 20, 2023

Conversation

kaisalmen
Copy link
Collaborator

@kaisalmen kaisalmen commented Sep 13, 2023

Fixes #46

The configuration change detection in componentDidUpdate is not correct. It interprets model related updates as application configuration updates. This PR introduces better logic to the wrapper and thereby shortens the react-component code.

There are now three levels of update protection:

  • It is checked if the editor app config is changed
  • It is checked if only the code or if the model changed

@kaisalmen kaisalmen force-pushed the issue-46 branch 2 times, most recently from 1a2a73b to bb0b92f Compare September 19, 2023 08:08
@kaisalmen kaisalmen changed the title WIP: Improve configuration change detection Improve configuration change detection Sep 19, 2023
@kaisalmen kaisalmen marked this pull request as ready for review September 19, 2023 08:11
Copy link
Member

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

General feedback points. There's a large conditional that's built in editorAppBase.ts that can probably be expressed as a series of branches or [a,b,c].some(p => p).

packages/monaco-editor-wrapper/src/editorAppBase.ts Outdated Show resolved Hide resolved
packages/monaco-editor-wrapper/src/editorAppBase.ts Outdated Show resolved Hide resolved
packages/monaco-editor-wrapper/src/editorAppBase.ts Outdated Show resolved Hide resolved
Copy link
Member

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

:octocat: approved!

@kaisalmen kaisalmen merged commit 24b09ec into main Sep 20, 2023
3 checks passed
@kaisalmen kaisalmen deleted the issue-46 branch September 20, 2023 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI Flickering after updating manually the content (code) of the editor
2 participants