Post template: try to remove duplicate preview #60535
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
1 similar comment
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -191 B (0%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
|
This would likely also resolve #58889. |
7 tasks
andrewserong
reviewed
Apr 7, 2024
There was a problem hiding this comment.
I've added a bit more context over in the other PR in #36431 (comment), but I believe we need to only render the inner blocks for "real" once for one of the instances of the loop. Otherwise we get duplicate controls everywhere:
Also, it looks like this winds up crashing the editor when attempting to browse the Query Loop patterns. The modal is at first slow, but if you go to scroll a bit or wait for a while, the page becomes unresponsive:
It's a good idea to revisit this block and see what can be optimised for its rendering, though! Some things to be mindful of when making changes here:
- There should only be one "real" rendering of inner blocks in an editable way. Otherwise we get duplicate controls showing. (Unless there's an alternative way to support
useInnerBlocksPropsbeing output in multiple components for the same block, without duplication / the risk of infinite loops) - Users should be able to click between each instance within the query loop, and there should be no flickering or layout jumpiness when switching between the instances. (The current duplication is there to prevent flickering when switching between a "real" instance of the loop, and a preview of the instance)
- Ideally, if a user is within one instance of the loop and clicks to a block that is within another instance of the loop, they'll be selecting that block instead of the parent post template block.
Hope that helps! As I said on the other PR, my memory is a little fuzzy when it comes to this block and the UI issues I helped fix at the time, but those points are the main things I can remember about the block.
|
@andrewserong Thanks for the clarifications! I'll close this PR for now and revisit it later. This list of requirements will help in that case. Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Try to understand #36431 and see what tests fail. I don't understand why we're rendering everything twice.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast