Add widgets customize inspector #29755
Conversation
kevin940726
added
the
[Feature] Widgets Customizer
|
Size Change: +2.52 kB (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
kevin940726
force-pushed
the
add/customize-widgets-inserter
branch
from
c02051e to
21fae3c
Compare
kevin940726
force-pushed
the
add/customize-widgets-inspect-panel
branch
from
33a7bc5 to
b78f171
Compare
|
I've tested this and it works great for a preview :) Code wise it looks pretty straightforward, but I suspect more complication will appear, considering how, for instance, clicking on a font size select in the inspector triggers a form data loss warning. But as it is now it's a great start 👏 Who knows maybe the plumbing will be minimal! |
There was a problem hiding this comment.
Nice one @kevin940726!
I don't think it's hacky at all 🙂 It's making good use of existing Customzier code using tools like createPortal which exist for exactly this use-case: a React app that is embedded into a non-React app.
The only issue I see is that there's a flicker in the animation because we hide the block editor before the transition.
packages/customize-widgets/src/components/sidebar-block-editor/index.js
Outdated
Show resolved
Hide resolved
packages/customize-widgets/src/components/inspector/block-inspector-button.js
Outdated
Show resolved
Hide resolved
| speak( __( 'Block settings closed' ) ); | ||
| } else { | ||
| speak( | ||
| __( |
There was a problem hiding this comment.
Hmm. Announcing this makes sense for the post block editor where the inspector appears off to the side where a person using a screen reader wouldn't notice it. In the Customizer, though, we hide the block editor and show the inspector. It's similar to page navigation.
I think that we should remove this speak() stuff and instead focus the first focusable element in the inspector when the inspector is opened, but cc. @tellthemachines or @talldan for opinions on whether there's a better pattern to follow.
There was a problem hiding this comment.
Ahh yeah that makes sense. I just copied most of the code from post editor without thinking much of it.
We should also hide the block editor when navigating to the inspector, but not unmount it. It's a little bit different from how the customizer's sections work now though, since mounting/re-mounting a block editor takes a significant amount of time.
There was a problem hiding this comment.
I think that we should remove this speak() stuff and instead focus the first focusable element in the inspector when the inspector is opened, but cc. @tellthemachines or @talldan for opinions on whether there's a better pattern to follow.
Sounds like you might need to decide whether it's a popover. It's kinda sounding like a popover.
If it's a popover:
- the opening button will have
aria-expandedandaria-haspopupspecified correctly - It'll focus when first opened
- Keyboard navigation would be constrained in the inspector
- Escape would close it.
- Focus loss would close it.
If it's a popover that works correctly I imagine you won't need the speak calls.
There was a problem hiding this comment.
This UI pattern already exists in the Customizer so I think let's match that as best as we can.
packages/customize-widgets/src/components/inspector/block-inspector-button.js
Outdated
Show resolved
Hide resolved
packages/customize-widgets/src/components/sidebar-block-editor/index.js
Outdated
Show resolved
Hide resolved
@draganescu It's because that the button by default has the |
|
Just discovered 2 more issues:
Added to the PR description. |
|
I noticed the block card has some styling issues (spacing, font-size, color) when compared to the block editor:
-- In other parts of the Customizer, the --
Blocks with variations include a dropdown in the inspector that allows you to switch to a different variation. In the screenshots above you can see that this dropdown is being cropped; Also, it doesn't seem to actually work, either. |
|
This is ready for another round of review!
These two are a little bit tricky though, I plan to fix them in another PR. 🙇 |
|
This PR has |
I set it as the base just to make reviewing easier, I don't think it depends on the base though. We can rebase it to trunk before merging. |
|
Looks good. Thanks @kevin940726. Go ahead and merge after you've rebased onto |
Co-authored-by: Robert Anderson <robert@noisysocks.com>
kevin940726
requested review from
cameronvoell,
chrisvanpatten,
dmsnell,
draganescu,
guarani,
gziolo,
mkaz,
mkevins,
mmtr,
nerrad,
noahtallen,
oandregal,
ntwb,
spacedmonkey,
swissspidy,
tellthemachines and
TimothyBJacobs
as code owners
kevin940726
force-pushed
the
add/customize-widgets-inspect-panel
branch
from
90c8e35 to
d618fe7
Compare
kevin940726
changed the base branch from
add/customize-widgets-inserter
to
trunk
| @@ -1,4 +1,15 @@ | |||
| .block-editor-block-inspector { | |||
| p { | |||
There was a problem hiding this comment.
It feels like adding so general styles to the BlockInspector might have unpredicted consequences on the UI outside of the Customizer.
There was a problem hiding this comment.
I remember seeing these styles before elsewhere in the codebase. They're styles that the Interface package would normally add:
https://github.com/WordPress/gutenberg/blob/trunk/packages/interface/src/components/complementary-area/style.scss#L34-L43
They probably shouldn't be there either.
There was a problem hiding this comment.
Yeah, these are the styles already applied to the inspector from <ComplementaryArea>. I was just trying to match the style here so that the inspector can have a consistent look when placing outside the complementary area.
Description
Close #29286
The inspector panel is done by manually inserting a fake section into the DOM via
createPortal. The transition effect is achieved by manually triggering classes inuseEffect. The implementation is pretty hacky, I'm not proud of it..., but it works? At least for now. We should also handle the i18n and a bunch of other things though, but I want to get a first round of review, in case I missed something really obvious 😅.TODOs
How has this been tested?
Screenshots
Kapture.2021-03-12.at.11.17.59.mp4
Types of changes
New feature
Checklist: