-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
update(IText): Add method _enterEditing #10190
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
src/shapes/IText/ITextBehavior.ts
Outdated
/** | ||
* runs the actual logic that enter from editing state, see {@link enterEditing} | ||
*/ | ||
protected _enterEditing() { |
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.
if is protected you can't really call it by yourself. This should stay public and have a different name.
Maybe enterEditingImpl ( as for implementation ) is better than the underscore prefix?
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.
I found that other internal methods that do not trigger the fire event are all prefixed with an underscore, such as
_discardActiveObject/discardActiveObject and _exitEditing/exitEditing, which is why I named it this way.
I should also change the _exitEditing method to public.
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.
To unify, I changed the method name _exitEditing to exitEditingImpl.
Also please divide eventual tests for enterEditing for the 2 respective methods, under JEST. |
Test cases have been added, but I cannot test them locally. The reasons are as follows: |
Hopefully i didn't break anything. |
There was an unrestored mock in the previous tests |
Yes but you never know what people do. |
Easier to keep and remove later. |
Add method _enterEditing to prevent triggering text:editing:entered.
#10187