-
Notifications
You must be signed in to change notification settings - Fork 416
♻️Misc refacto #480
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
♻️Misc refacto #480
Conversation
83eb370 to
05aded1
Compare
| useEffect(() => { | ||
| setHeadings(editor); | ||
|
|
||
| editor?.onEditorContentChange(() => { | ||
| setHeadings(editor); | ||
| }); | ||
|
|
||
| return () => { | ||
| resetHeadings(); | ||
| }; | ||
| }, [editor, resetHeadings, setHeadings]); |
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.
We already use it twice in exactly the same way, why not create a hook?
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.
Where ?
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.
Good catch ^^
Some logs were missing or not at the good place. This commit replaces them correctly.
We will have 2 urls targeting the server, better to improve the naming to avoid confusion.
Version are not editable, we don't need to activate the collaboration provider for them. Simplify the code by removing the provider from the version.
We add a check to be sure all the services are ready before starting the e2e tests.
05aded1 to
31ee9ce
Compare
- We create the useHeadings hook to manage the headings of the document and staty DRY. - We use the headings store in IconOpenPanelEditor and TableContent, to avoid prop drilling. - We add a debounce on the onEditorContentChange to improve a bit the performance.
31ee9ce to
8508fcd
Compare
Purpose
The PR #472 is quite big, we split what we can to make the review as easy as possible.
Proposal