Skip to content
New issue

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

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

Already on GitHub? # 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
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants