-
Notifications
You must be signed in to change notification settings - Fork 111
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
feat: hide context menu when onRenderContextMenu return false #411
feat: hide context menu when onRenderContextMenu return false #411
Conversation
Thanks! I think this is exactly the logic we need. I have a few feedbacks regarding the code:
|
Thanks, I've made the changes you suggested. |
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.
Niice job!! This looks really good, and keeps the behavior backward compatible.
I made two small inline comments, can you have a look at those?
Resolved |
Thanks! I was thinking: this still has to be published as a breaking changes: when you had an |
That's right, just like the adjustments made in the development page, this should really be known by those who are or have used |
… with a prop `readOnly` (#411)
I had one more thought: now that the context menu can be visible when the editor is readOnly, all the buttons should reckon with it and be disabled when not applicable in read-only mode. I've worked that out in c66ee09. I also came a cross a few minor issues related to the context menu, fixing that too. Also, I added a property |
This is an optimized version of pr #399