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

Small update on the website playground #2616

Merged
merged 3 commits into from Aug 23, 2021
Merged

Small update on the website playground #2616

merged 3 commits into from Aug 23, 2021

Conversation

Xoffio
Copy link
Contributor

@Xoffio Xoffio commented Aug 16, 2021

Hi.
I added 3 small fixes. Two of them are spelling corrections and the other one is an outdated comment
We can't dispose addCommand anymore... It can create confusion .

@ghost
Copy link

ghost commented Aug 16, 2021

CLA assistant check
All CLA requirements met.

@@ -7,4 +7,5 @@ var myBinding = editor.addCommand(monaco.KeyCode.F9, function() {
alert('F9 pressed!');
});

// When cleaning up remember to call myBinding.dispose()
// You can't dispose `addCommand`
// If you need to dispose it you might use `addAction` or `registerCommand`
Copy link
Member

Choose a reason for hiding this comment

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

I don't know all about addCommand / addAction, but why would you need to dispose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using monaco-editor in a NextJS app.
I need it to be updated every time an specific hook updates. If I don't dispose it the app gets slow pretty quick.
Now I'm just using addAction because it can be dispose.
It seams like it used to return a dispose then it change and someone forgot to update the playground.

Copy link
Contributor Author

@Xoffio Xoffio Aug 17, 2021

Choose a reason for hiding this comment

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

Here I opened an issue on monaco-react . At the beginning I thought I could use addCommand because in the playground you can see the lines:

// When cleaning up remember to call myBinding.dispose()

But, that is not true anymore. That's why I changed.

@alexdima alexdima added this to the August 2021 milestone Aug 23, 2021
@alexdima
Copy link
Member

Thank you!

@alexdima alexdima merged commit 848d520 into microsoft:main Aug 23, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2022
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.

None yet

3 participants