Allow moving metaboxes to previously empty area #25187
Conversation
|
Size Change: -24 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
the
[Type] Bug
|
Should we remove Also, I noticed that you forgot to update the |
|
|
|
Okay, but there's no reason to grab the |
|
Makes sense... pushed an additional commit 👍 |
|
Hmmm... I expected this to allow me to drag-and-drop metaboxes from the footer area to the sidebar area. I still can't, but I can use the new up/down buttons (introduced in WP 5.5) to move from the sidebar to the footer (and vice-versa), which I couldn't before. So this is definitely an improvement.
One issue I noticed is that moving from the footer area to the sidebar area doesn't work if the Document Settings sidebar is closed. But I wouldn't consider that a blocker. I did notice some more odd behavior, but I'm pretty sure it's caused by ACF, not WordPress itself. The weird behavior is that sometimes, pressing up/down on a metabox seems to do nothing. I think it has something to do with ACF altering the editor to have two footer areas (
Overall, I think this PR looks good to merge, but I'd like to get one more technical review just in case. |
|
Thank you for testing and reviewing this @ZebulanStanphill!
Correct. Adding a drag dropzone to something that doesn't technically exist visually would complicate this a lot and has the potential to cause more issues than it would solve. Using the up/down arrows is the only way to move blocks to an empty area, and since we now have the buttons it makes sense...
Yeah, that part is indeed coming from ACF 😅 |
aristath
requested review from
gziolo,
ellatrix,
noisysocks,
aduth and
ZebulanStanphill
But... something like this already works (quite well!) on the WordPress admin Dashboard page (see image attached).
Couldn't we make the 'Drag boxes here' pop-in on-drag when no box exists ( |
|
Something I've noticed here is that:
This is unexpected. It should either / or:
|
|
Looking forward to this fix, thanks y'all. |
|
Riad perhaps you could also take a look at this one? Associated: |
|
Hey @ribaricplusplus Bruno As you worked on Perhaps you might be able to take a closer look at this PR and help get it merged? |
|
Hi there! I resolved conflicts between trunk and this PR so the fix works again. There are just 2 things: Drag and drop It would be great if we could make drag and drop work, especially because it's not too difficult. In fact it's just a style issue, drag and drop doesn't work when there are no meta boxes because the drop zone height is 0. When I increase the drop zone height through dev tools (selector The only problem is that always having this extra height doesn't look nice, so maybe we could listen for UI sortable events and only add height when drag starts: https://api.jqueryui.com/sortable/#event-activate Can't move between location with arrows when Post sidebar hidden This was mentioned in this comment.
It would be awesome if we could fix this, but if not let's at least create a separate issue for the problem. The fix may require modifying WP Core postbox script where the click on those arrows is handled (https://github.com/WordPress/wordpress-develop/blob/a1a11d6cebd379e8ba95727fafd945f483f5114c/src/js/_enqueues/admin/postbox.js#L98), I'm not sure. Other than that, everything looks good to me. |
|
Perhaps @gwwar @talldan @andrewserong or @aristath can help with the last remaining pieces here (give some advice etc). The PR seems almost ready to land. |
|
Thank you for picking this up @ribaricplusplus and updating the PR 👍 |
|
I agree, there is no reason not to merge this. But maybe we should leave #7960 opened until we get drag and drop working, I'm leaving that up to you to decide. I created a separate issue for the arrows problem. |
|
What followups have you created @ribaricplusplus Bruno? |
|
I wrote the relevant summary here #7960 (comment) So the follow ups are:
|
andrewserong
changed the title
|
anyone else stumbles on this, admin css: addresses the dropzones being 0, but also only when you're actually dragging stuff. |
Description
Fixes #7960
In cbf425b we added
isVisible. The value of that does 2 things:The problem with the current implementation is that since an empty area doesn't get rendered on initial page-load, it's impossible to move blocks to a new metabox area that doesn't exist.
This PR changes the behaviour above so empty metabox areas will now get added to the DOM - but they will still be visually hidden. This change allows metaboxes to be moved to another metabox-area, even if that was previously empty (and therefore not visible and rendered with the previous implementation)
How has this been tested?
Tested by installing ACF and adding a metabox area on the side.
Before this commit the metabox could not be moved anywhere. After this commit it is possible to move it.
Types of changes
Removes the
isVisiblecondition from metabox rendering.Checklist:
My code follows the accessibility standards.My code has proper inline documentation.I've included developer documentation if appropriate.I've updated all React Native files affected by any refactorings/renamings in this PR.