List: use nested blocks #42711
Merged
List: use nested blocks #42711
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
|
Size Change: -880 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
oandregal
added
the
Needs Dev Note
|
Similarly to quote #25892 (comment) I'm adding a "needs dev note" to document this. While there're no markup changes the fact that list items have now a "data-block" attribute may cause issues in the editor due to theme authors using that to detect blocks. See #42526 (comment) |
ellatrix
force-pushed
the
enable/list-v2
branch
from
9d9f902 to
e484348
Compare
ellatrix
added
the
Mobile App - i.e. Android or iOS
ellatrix
force-pushed
the
enable/list-v2
branch
2 times, most recently
from
4b7b56f to
5a1a7cb
Compare
|
I've tested a bit the interactions and this is what I've found not working. None of them seems blockers for landing this, follow-ups would work. I still need to do the code review. See full list of tests
Things that I ran into:
220822-list-v2-cut-paste-into-new-block.webm
220822-merge-into-heading.webm
220822-merge-into-list.webm
220822-merge-two-lists.webm |
oandregal
reviewed
Aug 22, 2022
|
Thanks for testing! Let's follow up with any non blocker fixes and also add e2e tests for all of these. :) |
oandregal
reviewed
Aug 22, 2022
| shiftKey || | ||
| altKey || | ||
| metaKey || | ||
| ctrlKey |
There was a problem hiding this comment.
Oh, I just saw this. So this is how indent/outdent now currently works:
- indent: hit space at the beggining of the list item (either empty list item or item with text)
- outdent: hit backspace at the beginning of the list item
I remember that there were conversations about certain shortcuts conflicting with browser's shortcuts but don't remember the details. Perhaps that's why the CTRL+M one was removed?
There was a problem hiding this comment.
Yes, ctrl+m conflicts with minimise on mac.
oandregal
reviewed
Aug 22, 2022
oandregal
reviewed
Aug 22, 2022
oandregal
reviewed
Aug 22, 2022
| @@ -1,177 +1,188 @@ | |||
| /** | |||
There was a problem hiding this comment.
The whole edit function is simplified a great deal with these changes! 👏
oandregal
reviewed
Aug 23, 2022
oandregal
reviewed
Aug 23, 2022
oandregal
reviewed
Aug 23, 2022
oandregal
approved these changes
Aug 23, 2022
There was a problem hiding this comment.
I've finalized the code review: there's a couple of tweaks we can do and I think I've noticed a couple of other gotchas (transform to TOC block, etc). They are small enough for me to approve and either address here or in follow-ups.
I've also tested the paste transformation and that blocks created with the old version were properly transformed into the new one.
Let's merge this and polish
ellatrix
force-pushed
the
enable/list-v2
branch
from
a144874 to
ac35356
Compare
geriux
approved these changes
Aug 24, 2022
|
Rebased to get #43571 in and avoid unrelated e2e test failures. |
MaggieCabrera
pushed a commit
that referenced
this pull request
Aug 25, 2022
Co-authored-by: André <[email protected]>
@oandregal Not sure I understand why it should be converted into a heading. Backspace should just unwrap the text, not convert it to anything.
There's already a PR for this: #43181
Created an issue: #43607 |
Makes sense. Thought to bring it up because the list v1 did convert the items into heading blocks: Gravacao.de.ecra.a.partir.de.26-08-2022.12.27.49.webm |
|
The test content used with the performance test still uses the old markup for the List block. It could impact the "First block loaded" results because those blocks need to be migrated to the new markup.
Source: https://codehealth.vercel.app File to update: I applied a similar update after the Quote block with nesting landed #43359 but it didn't have a bigger impact. There is also a noticeable increase in the "Typing" time result with this PR introduced, which might be simply an effect of using inner blocks:
Source: https://codehealth.vercel.app |
|
Created a PR to update the test content at #43770 |
|
I do see a big increase in the
Waiting for the results of the performance tests at #43770 to see if that's attributable to the migration hook (transform list v1 to list v2). |
38 tasks
|
With the test content updated, it looks like the latest speed tests are still about 50% slower than 6.0: https://make.wordpress.org/core/2022/10/13/whats-new-in-gutenberg-14-3-12-october/ |
This was referenced Aug 15, 2023
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?
Enables List v2.
Fixes #6394.
Fixes #21406.
Fixes #32169.
Fixes #40348.
Fixes #17052.
We need to wait with this PR until v2 is released for mobile, so do not merge yet.
Remaining important things to fix
Why?
Each list item will be a block, and each nested list will also be a separate block with separate settings.
Among other things, it allows list items to be easily ordered.
How?
With
InnerBlocks!Testing Instructions
Test the list block generally. The e2e tests should cover us.
Screenshots or screencast