[Widget Editor] Fixed sidebar inserter #25681
Conversation
adamziel
added
[Feature] Widgets Screen
|
Size Change: +1.37 kB (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
| onMouseDown={ ( event ) => { | ||
| event.preventDefault(); | ||
| } } | ||
| onClick={ () => { |
There was a problem hiding this comment.
Nitpick: Let's move it to a function in the render function to make easier to read.
| true | ||
| ); | ||
| } | ||
| // eslint-disable-next-line @wordpress/react-no-unsafe-timeout |
There was a problem hiding this comment.
Without it, eslint complains about doing setTimeout without matching clearTimeout. In this case the setTimeout call is all about running code on the next tick so clearTimeout wouldn't make as much sense.
There was a problem hiding this comment.
Ahh I meant to comment the next line: why do we need the setTimeout?
There was a problem hiding this comment.
Without it, the DOM updates resulting from selectBlock() and setIsInserterOpened() calls are applied the same tick and pretty much in a random order. The inserter is closed if any other part of the app receives focus. If selectBlock() happens to take effect after setIsInserterOpened() then the inserter is visible for a brief moment and then gets auto-closed due to focus moving to the selected block.
There was a problem hiding this comment.
Maybe requestAnimationFrame would be a better choice? And would also appreciate if you can add some comments above it, since it's not idiomatic code 😅
| withFocusReturn( ( { children } ) => children ) | ||
| ); | ||
|
|
||
| export default function PopoverWrapper( { onClose, children, className } ) { |
There was a problem hiding this comment.
This is a very generic component and I think ideally this becomes a set of hooks or a single hook as it's more about behavior and less about UI. (that said, it's separate from this PR)
|
@adamziel while testing I found a problem:
|
|
@kevin940726 interesting! I played with it a bit and failed to reproduce the issue - how did you arrive at that state? |
@draganescu it's not a bug, it's a feature. The inserter acts like a popover, just like it does in the post editor. We discussed some related problems in #24561 and #24822. Tl;dr - we concluded that following the post editor pattern is the most sensible way to start. |
I agree. This is correct behavior. Although I noticed that the first click on another widget area did not immediately open that area. It should. Here's what's happening now:
The other issue I noticed, which I'm not sure should be posted here, but here goes... The drop placeholder when hovering over the blocks in the Inserter panel should appear BELOW any existing blocks if nothing is selected. Same way it works in the post editor. Notice below, I already have a block in the first area, but the drop placeholder is appearing above it.
|
|
@kevin940726 nevermind, I just reproduced it and added a fix (overflow: hidden). It's not perfect but should do it for this PR and 9.1. Let's absolutely revisit it. |
There was a problem hiding this comment.
Because this advances the UI a lot in regard to the design spec we have I'd like to merge this and come back to it's problems in future PRs.
We'll need to address:
- The widget areas should expand in one click
- The insert highlight in widget area should show at the end of the area
- Remove the need to have
overflow:hiddenfor.block-editor-inserter__menu
|
Worth noticing that if the expected behavior is to append the block to the end of the widget area, we might don't need |
* Fixed sidebar inserter * Styles for the title * Scrollable toolbar on mobile * Add a comment and extract handleClick into a separate function * Fix the page height issue
Description
Solves #25649
Side-note: As the fixed sidebar inserter may also be useful in the navigation editor, and other editors yet to come, it could be a good idea to extract reusable parts to the interface package.
How has this been tested?
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: