-
-
Notifications
You must be signed in to change notification settings - Fork 648
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(facade): add lifecycle hooks for facade #2357
Conversation
Origin Title: feat(facade): add lifecycle hooks for facade Title: feat(facade): add lifecycle hooks for facade Why add lifecycle hook? Documents to be added Pull Request Checklist
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2357 +/- ##
=======================================
Coverage 29.25% 29.26%
=======================================
Files 1515 1516 +1
Lines 76698 76711 +13
Branches 15929 15929
=======================================
+ Hits 22441 22449 +8
- Misses 54257 54262 +5 ☔ View full report in Codecov by Sentry. |
View Deployment
|
/** | ||
* Get hooks | ||
*/ | ||
getHooks() { |
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.
The name should be more specific, like getLifecycleHooks
. Unless you're going to bind other types of hooks o f-hook
. (And what are other possible books?)
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 have considered this, and it is indeed worth discussing.
The current design is divided into 4 types of hooks, FHooks general hooks, FSheetHooks/FDocHooks/FSlideHooks hooks for specific document types
Although the role of hooks can be replaced by listening to Command in most scenarios, hooks are more semantic and more popular. In the future, we may refer to hansontable to add some hooks https://handsontable.com/docs/javascript-data-grid/api/hooks/
Add afterDispose, afterCopy, afterPaste... to FHooks
This brings up a management issue. Should the internal distinction of the 4 types of hooks be refined?
If it is necessary to refine, it can be divided into
FLifecycleHooks: including life cycle, dispose event
FClipboardHooks: clipboard hook
FRowHooks
FColumnHooks
FCellHooks
etc.
Which solution is better?
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.
OK. Let's merge all hooks in this class. Maybe it's better for new users to get what they want.
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.
LGTM
为什么要新增 lifecycle 钩子?
注册 canvas 扩展的 Facade API,必须在渲染完成之后才能调用,可以抛出生命周期的钩子,让用户知道已经渲染完成,再调用 canvas 扩展的API
文档待补充
Pull Request Checklist