-
Notifications
You must be signed in to change notification settings - Fork 562
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
#1332: Performance improvements. #1350
Conversation
Amazing @haringsrob! Thanks for flagging what to look for. Regarding the It might be that this is not necessary anymore, but worth testing further. |
|
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.
This is awesome @haringsrob, quite a big gain from a relatively small fix!
One quick question below re. beforeDestroy()
to make sure I understand correctly.
@@ -103,12 +103,5 @@ export default { | |||
// init value with the one present into the component itself | |||
this.saveIntoStore() | |||
} | |||
}, | |||
beforeDestroy: function () { |
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.
Just wanted to echo the comment from @ifox. With this change, if I add a block, fill some fields, then delete the block, is this content still sent to the backend on save?
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.
Yes I checked, but I will test it once more tomorrow to be sure.
Add/remove edit all should work fine in all languages.
Having another look at this today it seems to break reference fields, media fields |
I double checked this and resolved some issues. For all destroy hooks I checked and the db is clean after removing an item. |
Description
This pr changes a bit when the frontend renders fields.
For block editor: It will only render the fields if the item is expanded. Freeing up dom space.
For fields: It will only render the active language fields, Freeing up dom space.
This was tested on a page with 100 blocks, each having 102 fields and 8 languages.
After this pr:
Time to interactive (via lighthouse): 3.2 s
Dom size: nodes: 2972, depth: 17, length: 188737
Before this pr (had to reduce the languages to 2 to make it work):
Time to interactive (via lighthouse): 8.3 s
Dom size: nodes: 45331, depth: 31, length: 410479
So this pr dramatically reduces the size of the page and improved the performance by quite a lot on bigger blocks editors/language combinations.
Please do have a look at this as I don't know why it is there or what side effects it may generate:
https://github.com/area17/twill/compare/2.x...haringsrob:1332-dom-performance?expand=1#diff-4d78aaef72f437ac2e1e4230b2bf68cc575aa52a7ec4798ecf659ef89768f71f
Related Issues
Fixes #1332