Query Loop: Allow "enhanced pagination" only with core blocks #54347
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
24 tasks
|
Size Change: +1.5 kB (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
luisherranz
added
[Block] Query Loop
luisherranz
requested changes
Sep 11, 2023
There was a problem hiding this comment.
I haven't reviewed the code yet, but the UX feels great so far, congratulations 🙂
However, we need to check that there are no third-party blocks inside the Query block, not inside the Post Template block because the regions covers all the Query block nodes.
Can you change that?
Screen.Capture.on.2023-09-11.at.14-46-46.mp4
By the way, it's not up to me to decide, but I think showing a modal is the correct UX. Preventing blocks from being inserted could be confusing. |
|
For context, we're still thinking about the mechanism to identify compatible blocks. More information here: But until we make a decision, we decided to limit the feature to only the Core blocks because they are the most common ones and we know that they are all compatible. |
Sure, it makes total sense! I think I read the task description too literally. 😅
|
|
That was my bad, sorry 🙂 |
|
Flaky tests detected in 72bf6ff. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6149963302
|
luisherranz
requested changes
Sep 12, 2023
There was a problem hiding this comment.
We need to improve the accessibility of the modal. I made a video to explain, but basically I think that when the modal appears, the focus should be on the Ok, understood button, and the title & message of the modal should be announced.
https://www.loom.com/share/4398656726654e8db6cac53b5709d209
EDIT: Can you search for all the modals in the codebase to see if there's another one that appears suddenly like this one, to see how they deal with the accessibility?
|
Thanks for the review, @luisherranz. 🙏 Yup, I also noticed the accessibility issues with the Modal component. I'm working on fixing them. |
|
Apart from that, do you consider this ready for review? |
21 tasks
@luisherranz, I've changed my mind; I'll address those changes in a different PR. Ready for review. 🙂 |
6 tasks
6 tasks
DAreRodz
deleted the
update/query-loop-enhanced-pagination-only-with-core-blocks
branch
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?
Prevents enabling enhanced pagination in a Query Loop containing any third-party block. This ensures block compatibility.
The block follows this logic:
Tracking issue: #54291
Why?
Third-party blocks can be interactive and use a different library than the Interactivity API. This could lead to problems as the
Interactivity API can add, modify, or remove DOM nodes handled by third-party libraries, thus making the blocks that depend on those libraries stop working.
How?
I haven't found an easy way to prevent certain blocks from being inserted inside another at any level, so I wrote a hook to detect what kind of blocks are contained. If a third-party block is found, and the setting is enabled, a modal appears, requiring action to keep a valid state.
Testing Instructions
The feature is not stable yet, although the UX can be tested:
Testing Instructions for Keyboard
TBD
Screenshots or screencast
Screen.Recording.2023-09-11.at.19.44.16.mov