-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
List block v2: Add start, ordered and reversed list block attributes #39605
Conversation
Size Change: +113 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
Nice! One small note, I tested creating a list first, then enabling the experiment, which then invalidated the block. I imagine once we're happy with the state, this will be tackled? Taking the PR for a spin, a few small things:
A subtle rich text behavior appears when deleting a parent item: In the above example, I would have expected that the child list became a child list of item 5 instead of disappearing. Is there anything we can do here? In nitpick territory, there's some spacing with the block toolbar here: The ellipsis is too wide, and the separator sits in an odd place. I wonder if it's the first-child/last-child rules that govern the toolbar messing things up: it'd be nice to refactor the block toolbar to use flex and gap here. I know, big diversion, probably 🙈
In terms of lower-hanging fruit, we could potentially revisit #21477 so the absorb toolbar mechanism (which goes in both directions now IIRC) could scale better. There's the larger question: how do we handle this gracefully in nested rich text blocks generally, including blockquote and the likes. A while back I explored a prototype for the Quote block: We ended up implementing the parent button as a starting point, as that was also an existing ticket and concept. But for the sake of revisiting, and keeping toolbars as contextual as possible, I wanted to just see what that might look like for the list block. Quick and dirty revisit of that prototype: The primary benefit of keeping the parent button in context of the parent is that it affords a bit of a shorter toolbar. Revisiting these, I'm not sure that's worth it. Most pragmatic change is probably to consider what goes in the toolbar when in absorbed mode. Here's an idea: In this one, the list style type is a dropdown, that merges into the list item "text alignment" segment when children are selected. I also hid the "List item" icon, and I'm unsure if that was a good idea. But when there are no transforms for list items, the icon is just inert and does nothing, do we need to show it? Maybe probably, the above visualizes the "what if" of not showing it. |
yes #39521
This has its own issue #39516
I agree, I think we can at least remove from this block once all the keyboard interactions are in place
Maybe, but that's not right text or anything related. I guess this is part of #39519
I'll debug and share more
For me, this is going into the right direction, there's also a question to be asked about the inspector (colors, start, reversed config) when the child is selected. We should also try to answer this holistically:
I don't know about this either, We can hide it if needed but we also need to come up with the right heuristics. These bits of UI are generic and not block specific. |
yep, the issue here is the last-child is not applying to the "button" element because there's a hidden div showing the description of the button after the button. I guess we can try block gap but it probably deserves its own PR to account for all the use-cases. |
I can't recall the heuristics, but I think @gziolo did some explorations here. |
@jasmussen I tried looking at the block toolbar thing and I'm not really sure how you'd like to use block gap to solve this issue. Do you have a codepen or something? |
Yeah let me whip up a codepen to figure that out! |
Alright, how's this? https://codepen.io/joen/pen/KKZMGmB?editors=1100 |
@jasmussen The problem with this is the small click area. |
Oh right, good point. I'll see if I can't redo it! |
This one has identical metrics: https://codepen.io/joen/pen/LYeZqep?editors=1100 I like that one a lot, because all buttons have the same size, except for the drag handle, which is denoted as "compact". Gap is no longer used, but the button paddings, and the section padding achieves the same. What do you think? |
I like the consistency of it, I guess some might argue that the one button groups are not full width now but for me it's fine. |
Even in those cases, the pressable area is 36x48px, which is big. If it ever becomes a problem, and if we can get the markup to be consistent and predictable, we could do something with Overall, though, I think the potential code refactor is worth it. |
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.
LGTM! Thanks Riad!
I think we can land and handle the toolbar icons issue in a separate PR. This actually exists in current list block and I hadn't noticed before 😄
closes #39518
What?
The list block is being refactored and rewritten to use "inner blocks" instead of a single block. As part of that project, this PR brings the list ordered, start and reversed attributes configuration from the v1 into to v2 of the list block.
How?
The nice thing here is that we don't have to extracted these informations from the "RichText" value like in v1, we can use the dedicated attributes and it will work in a nested way.
Right now though, the controls are only visible when the "list block" is selected as this embraces the inner blocks behavior. That's probably a decent start but I guess we can consider some "aborbToolbar" (or opposite here). What would be the ideal UX here cc @jasmussen @mtias.
Testing Instructions
Screenshots or screencast
Screen.Recording.2022-03-21.at.8.31.17.AM.mov