Use Card instead of Panel for the block editor #22827
Conversation
|
Size Change: +84 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
adamziel
changed the title
adamziel
changed the title
| <CardHeader className="edit-navigation-menu-editor__block-editor-panel-header"> | ||
| { __( 'Navigation menu' ) } | ||
| </CardHeader> | ||
| <CardBody> | ||
| <div className="components-panel__header-actions"> | ||
| <Button isPrimary onClick={ saveBlocks }> |
There was a problem hiding this comment.
I think it would make sense to put the <Button> component up within the <CardHeader> component, and maybe position it to the right-side of the header.
| <CardHeader className="edit-navigation-menu-editor__block-editor-panel-header"> | ||
| { __( 'Navigation menu' ) } | ||
| </CardHeader> | ||
| <CardBody> | ||
| <div className="components-panel__header-actions"> |
There was a problem hiding this comment.
Whats the purpose of this .components-panel__header-actions element?
|
@shaunandrews while I was at it, I also moved the delete link to the top:
|
|
Feel free to ignore me, but that delete link at the top looks like an important message, a red thing at the top is panicky, and can be hit instead of save. |
|
Hm, good call. I'll move it back to the bottom for now and let's have a separate discussion about that. On the existing nav-menus screen it is actually at the bottom together with a second Save button:
|
|
This is the latest version:
|
|
I don’t think we need a duplicate save button at the bottom. |
|
@shaunandrews Some additional context is that one of the next action items for this screen is adding a switch between horizontal and vertical mode of displaying the menu and the save button at the bottom could be useful in the vertical mode. If you still recommending removing it, I'll do it - just LMK. |
|
Yes let’s remove the bottom button. Thanks. |
adamziel
force-pushed
the
update/replace-menu-editor-area-with-card
branch
from
c6a7912 to
e5a625e
Compare
|
@shaunandrews @draganescu This is ready for a re-review now.
|
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| } |
There was a problem hiding this comment.
should these be bundled in the Card component or not? Genuinely asking, I don't have the answer but if I understand properly when we find ourselves doing this kind of override (which we do often), maybe we're missing something in the abstraction. cc @ItsJonQ
There was a problem hiding this comment.
Thanks for the heads up @youknowriad !
We recently merged in an update that added a new layout-based component, <Flex />, that handles this very use-case!
https://github.com/WordPress/gutenberg/tree/master/packages/components/src/flex
I can update the <Card /> sub-components to use <Flex />, which should save us from having to write CSS like this in future use
Description
There is no reason to make the BlockEditorPanel collapsible, so let's use Card instead of Panel there. A follow-up PR will make the other panel only collapsible on mobile devices.
Before:
After:
Types of changes
Non-breaking changes
Checklist: