Uncollapse widget area when block is dragged over #25992
Conversation
adamziel
added
[Feature] Widgets Screen
adamziel
requested review from
noisysocks,
talldan,
paaljoachim and
kevin940726
adamziel
requested review from
draganescu,
tellthemachines and
youknowriad
as code owners
| ) } | ||
| </PanelBody> | ||
| </Panel> | ||
| <div onDragOver={ openOnDragOver }> |
There was a problem hiding this comment.
There are already a few blank <div/> elements in the tree so this shouldn't break anything. An alternative would be to do <Panel onDragOver={ openOnDragOver }> and then update <Panel /> to apply arbitrary properties to its root element, thoughts?
There was a problem hiding this comment.
Yep, I think this is ok.
Another alternative would be <Panel openOnDragOver> making the behavior implicit to Panel. Though I think given PanelBody is more of a controlled component it's better to keep it explicit like you have it here.
There was a problem hiding this comment.
Actually, I realise there's some block CSS that might need to be adjusted (though it's currently broken anyway)
|
Size Change: +317 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
|
Hey Adam (I am looking at the UI approach.) This is what I see: Dragging the widget over the Footer 2 closed widget area, creates a sudden jerk/push downward of the cursor. Here it would be better to calmly drag a widget over the closed area, and have the area open downward. It should not affect the drag only the area. Here is the old Widget screen in comparison: What is great about the old method is that it does nothing with the drag as it happens. |
|
@paaljoachim my thoughts exactly! That's what I meant by asking:
If the user wants to drag the widget from sidebar number 1 to sidebar 8, the current jerk will make it pretty hard so I think we should indicate that the area is about to open, but still give user a chance to keep dragging. I'm not sure how that indication would look like, hence I pinged @mapk :-) |
|
@paaljoachim I think your suggestion to makes collapsing/uncollapsing smooth would make a great feature not just here, but also for the Panel component. |
| [ clientId ] | ||
| ); | ||
| const { setIsWidgetAreaOpen } = useDispatch( 'core/edit-widgets' ); | ||
| const openOnDragOver = useCallback( () => { | ||
| if ( ! isOpen && isDraggingBlocks ) { |
There was a problem hiding this comment.
I don't think we should worry about isDraggingBlocks, it should also open for dragged files and HTML so that users can upload/add things like images that way.
| ) } | ||
| </PanelBody> | ||
| </Panel> | ||
| <div onDragOver={ openOnDragOver }> |
There was a problem hiding this comment.
Yep, I think this is ok.
Another alternative would be <Panel openOnDragOver> making the behavior implicit to Panel. Though I think given PanelBody is more of a controlled component it's better to keep it explicit like you have it here.
|
Hey 👋 there. I really like auto-expanding the area when dragging over it! I do also agree with @paaljoachim's feedback. The jump that occurs right now is confusing.
I don't think we need an indicator necessarily. The expansion of the area seems to be a sufficient. Moving the dragged block into a collapsed area should open it but retain its positioning on screen and not jump to a particular point. Dragging it back out of the area should collapse that area again. I like how it works in the Classic Widget screen that Paal shared. When opening up an area on drag, the blue line drop indicator should appear above any existing widgets in that area if the user is dragging downward from one area to another area below. This should reverse when dragging up from an area below to an area above. |
|
There is some similar code in the navigation-link block that might be reusable (cc @kevin940726), but still think this works well enough as a first iteration. |
|
I think it's nearly there, the last part is smooth panel animation - let's make it a separate PR. |
| * | ||
| * @return {boolean} Is dragging within the target element. | ||
| */ | ||
| const useIsDraggingWithin = ( elementRef ) => { |
There was a problem hiding this comment.
I just copied that over from edit-navigation - happy to move it to another package if there are any good candidates.
There was a problem hiding this comment.
Seems fine to duplicate it for now 👍
| // Ref is used so that the effect does not re-run upon scrollAfterOpen changing value | ||
| const scrollAfterOpenRef = useRef(); | ||
| scrollAfterOpenRef.current = scrollAfterOpen; |
There was a problem hiding this comment.
This seems hackish - any better ideas?
| } else if ( ! isDraggingWithin && isOpen && openedWhileDragging ) { | ||
| timeout = setTimeout( () => { | ||
| setOpen( false ); | ||
| }, 100 ); |
There was a problem hiding this comment.
Completely arbitrary timeouts
There was a problem hiding this comment.
I'm personally finding the delays are unhelpful. The problem is that there's no indication anything is about to happen when I first drag over, and also the way the page scrolls while dragging means it's quite easy to zoom past an area.
The sidebars for the old widget page open immediately, so I think we can replicate that for now and revisit this. I took the liberty of pushing a commit to do that.
|
I just gave this a go again! Really close! I'm still noticing a jump when dragging into a longer widget area. Can we just expand the widget area downward without the jump? I also saw that when leaving a widget area that expanded, it doesn't collapse again. Is this possible?
|
|
Testing this, I'm still unable to drag a widget into an empty widget area. Even though the empty now opens when dragged over, there's no drop indicator and dropping a widget on it doesn't work. How easy would it be to fix it in this PR? Or better to open another issue? |
|
@tellthemachines The fix was only recently merged - #26051 It's either that this PR needs to be rebased, or it might also need to be updated to account for the selectors mentioned here: |
|
I tested this and for me it did not work. |
|
I tested again. Either I am not testing correctly as it seems to be showing an older commit. This is what I see:
There is still a jumpiness when dragging from one widget area to another. As it jumps too far down. |
talldan
force-pushed
the
update/uncollapse-widget-areas-on-drag
branch
from
83919b5 to
04ef9a5
Compare
There was a problem hiding this comment.
I've rebased and double-checked the fix from #26051 still works, which it does 👍
I'm personally not seeing any sudden jumps with the latest code, but noticed a separate issue.
The scroll while dragging behavior that the block editor has isn't particularly great with panels. What happens is that when a panel opens while dragging over it, suddenly there's more space that the page can scroll into.
Not sure what to do about that, but it's definitely separate to this.
Let's merge and then iterate to improve it.
talldan
changed the title
| const scrollAfterOpenRef = useRef(); | ||
| scrollAfterOpenRef.current = scrollAfterOpen; |
There was a problem hiding this comment.
I think this could be simplified to:
| const scrollAfterOpenRef = useRef(); | |
| scrollAfterOpenRef.current = scrollAfterOpen; | |
| const scrollAfterOpenRef = useRef( scrollAfterOpen ); |
And yeah, this definitely feels like a code smell.
Description
Solves #25243
I wonder if we should add a timeout and some indication that the widget area is about to uncollapse @mapk ?
How has this been tested?
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: