Use interface on widgets screen sidebar#22304
Conversation
jorgefilipecosta
added
the
[Feature] Widgets Screen
|
Size Change: +258 B (0%) Total Size: 835 kB
ℹ️ View Unchanged
|
gziolo
left a comment
gziolo
left a comment
There was a problem hiding this comment.
Nice to see how those component integrates so easily. Great job @jorgefilipecosta 😃
For the full picture, I haven’t tested this PR.
| margin: auto; | ||
| } | ||
|
|
||
| .edit-widgets-sidebar .components-button.interface-complementary-area__pin-unpin-item { |
There was a problem hiding this comment.
Was the prop to disable pinning feature ported over from edit-post?
| className="edit-widgets-sidebar" | ||
| smallScreenTitle={ __( 'Widget Areas' ) } | ||
| scope="core/edit-widgets" | ||
| complementaryAreaIdentifier="edit-widgets/block-inspector" |
There was a problem hiding this comment.
I guess the renaming of this prop is in progress in other PR? The name is long and repeats mostly the name of the component.
| complementaryArea: { | ||
| 'core/edit-site': 'edit-site/block-inspector', | ||
| 'core/edit-post': 'edit-post/document', | ||
| 'core/edit-widgets': 'edit-widgets/block-inspector', |
There was a problem hiding this comment.
Why do we need these here? A third-party plugin implementing his own area won't have access to this.
There was a problem hiding this comment.
I assumed you were fine with the structure of the reducer 😃
There was a problem hiding this comment.
This line just ensures a sidebar is open by default when we load a screen for the first time ever. On the follow loads we persist the sidebar state from the local storage.
But it is a good point if a plugin is implementing its own screen, it would not be able to set a default.
I will try to solve this issue and remove these defaults in a separate PR. I have two options on the slot I can add a prop that sets the name of the sidebar open for the first time ever, or on the complimentary area (fill) I set a boolean flag if the complementary should be the one open for the first time. Any preference on these two options?
There was a problem hiding this comment.
So basically a choice between a prop on the "slot" or on the "fill" right?
There was a problem hiding this comment.
Slots probably won't know what fills they have (if they come from plugins), so it seems like a prop on the fill might be better. The question becomes what happens if multiple fills do that.
There was a problem hiding this comment.
The advantage of the slot is that we would not have the problem of two fills specifying that flag. The sidebars would be internal so the slot would be able to be aware of them. But I agree it makes more sense to be set on the fill my only fear is it ends up being used by the plugins to have their toolbar open by default. But on second thought they can already do that using our actions.
I will try the fill approach.
The question becomes what happens if multiple fills do that
I guess the first or the last one being rendered would appear open. But it is something we need to test in practice.
jorgefilipecosta
force-pushed
the
try/use-single-block-editor-for-widgets
branch
2 times, most recently
from
a1c15cb to
8746f6d
Compare
jorgefilipecosta
changed the base branch from
try/use-single-block-editor-for-widgets
to
master
jorgefilipecosta
force-pushed
the
update/use-interface-package-widgets-screen
branch
from
e7725a6 to
850f956
Compare
3 participants
Description
Branched from #22140.
This PR adds the complementary area mechanism from the WordPress interface package into the widgets screen.
This allows the sidebar to be toggled on and off like in the other screens.