This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add Product Image Gallery #8235
Add Product Image Gallery #8235
Changes from 13 commits
38fcacb
2a1b52c
5f889e6
13dc046
1dc7baa
21a1fea
d9900e0
75d00bb
f8e00c9
795a698
90c71d3
dc48ddf
e526eff
66595f1
c701d7f
5a4ac3b
8aee841
32f97dd
e3be328
b6a8eb5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could we use debounce here to make this trigger less frequent?
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.
Mmm, I would avoid a debounce here to avoid any kind of race condition.
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'm curious what can be the race condition and its drawback. The motivation behind my suggestion is to keep the editor performant.
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.
Mmm, for example, the user is editing another template, but the function isn't called due to the debounce and the block is still visible in the inserter.
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 don't think we will have this issue, as we just want to debounce them (by 100ms, for example), the subscription still triggers, but less frequently. So when the user switches to another template, the subscription may not trigger right away, but still fast enough to register/unregister the block before user interaction.
We don't have to do it now btw, we can go with the current PR and come back to this later when we see it affect the editor performance.