Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicate navigation block props #43596

Merged
merged 3 commits into from
Aug 25, 2022
Merged

Fix duplicate navigation block props #43596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37b8714
Fix duplicate block props in unsaved navigation inner blocks
talldan Aug 25, 2022
70de268
Fix broken layout
talldan Aug 25, 2022
100039f
Remove unused styles
talldan Aug 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/block-library/src/navigation/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,6 @@ function Navigation( {
overlayTextColor={ overlayTextColor }
>
<UnsavedInnerBlocks
blockProps={ blockProps }
blocks={ uncontrolledInnerBlocks }
clientId={ clientId }
navigationMenus={ navigationMenus }
Expand Down
38 changes: 14 additions & 24 deletions packages/block-library/src/navigation/edit/unsaved-inner-blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -42,7 +37,6 @@ const ALLOWED_BLOCKS = [
];

export default function UnsavedInnerBlocks( {
blockProps,
blocks,
clientId,
hasSavedUnsavedInnerBlocks,
Expand Down Expand Up @@ -83,12 +77,17 @@ export default function UnsavedInnerBlocks( {
const isDisabled = useContext( Disabled.Context );
const savingLock = useRef( false );

const innerBlocksProps = useInnerBlocksProps( blockProps, {
renderAppender: hasSelection ? undefined : false,
allowedBlocks: ALLOWED_BLOCKS,
__experimentalDefaultBlock: DEFAULT_BLOCK,
__experimentalDirectInsert: shouldDirectInsert,
} );
const innerBlocksProps = useInnerBlocksProps(
{
className: 'wp-block-navigation__container',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches innerBlocksProps elsewhere.

},
{
renderAppender: hasSelection ? undefined : false,
allowedBlocks: ALLOWED_BLOCKS,
__experimentalDefaultBlock: DEFAULT_BLOCK,
__experimentalDirectInsert: shouldDirectInsert,
}
);

const { isSaving, draftNavigationMenus, hasResolvedDraftNavigationMenus } =
useSelect(
Expand Down Expand Up @@ -172,18 +171,9 @@ export default function UnsavedInnerBlocks( {
const Wrapper = isSaving ? Disabled : 'div';

return (
<Wrapper className="wp-block-navigation__unsaved-changes">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There used to be an overlay to prevent direct manipulation, but I don't think this is needed anymore. It causes the styling of the inner blocks to be incorrect. There must be direct descendent selectors among the block styles, so I think it's best to try to match the DOM structure that the block normally has.

This could do with some testing, but I believe the unsaved inner blocks are save upon editing now, and it seemed to work ok for me. It's questionable if the Disabled wrapper is also still needed, but I left it there.

I did spot some issues that are also present in trunk, like the way links added after page list are aligned to the right:
Screen Shot 2022-08-25 at 3 03 19 pm

<div
className={ classnames(
'wp-block-navigation__unsaved-changes-overlay',
{
'is-saving': isSaving,
}
) }
>
<div { ...innerBlocksProps } />
</div>
<>
<Wrapper { ...innerBlocksProps } />
{ isSaving && <Spinner /> }
</Wrapper>
</>
);
}
20 changes: 0 additions & 20 deletions packages/block-library/src/navigation/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,21 +568,6 @@ body.editor-styles-wrapper
padding: $grid-unit-10 $grid-unit-15;
}

.wp-block-navigation__unsaved-changes {
position: relative;

.components-spinner {
position: absolute;
top: calc(50% - #{$spinner-size} / 2);
left: calc(50% - #{$spinner-size} / 2);

// Delay showing the saving spinner until after 2 seconds.
// This should ensure it only shows for slow connections.
opacity: 0;
animation: 0.5s linear 2s normal forwards fadein;
}
}

@keyframes fadeouthalf {
0% {
opacity: 1;
Expand All @@ -592,11 +577,6 @@ body.editor-styles-wrapper
}
}

.wp-block-navigation__unsaved-changes-overlay.is-saving {
opacity: 1;
animation: 0.5s linear 2s normal forwards fadeouthalf;
}

.wp-block-navigation-delete-menu-button {
width: 100%;
justify-content: center;
Expand Down