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.

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

Adds all allowed innerblocks to the inspector animation experiment #47834

Merged
merged 6 commits into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 11 additions & 14 deletions packages/block-editor/src/components/block-inspector/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { default as InspectorControlsTabs } from '../inspector-controls-tabs';
import useInspectorControlsTabs from '../inspector-controls-tabs/use-inspector-controls-tabs';
import AdvancedControls from '../inspector-controls-tabs/advanced-controls-panel';
import PositionControls from '../inspector-controls-tabs/position-controls-panel';
import useBlockInspectorAnimationSettings from './useBlockInspectorAnimationSettings';

function useContentBlocks( blockTypes, block ) {
const contentBlocksObjectAux = useMemo( () => {
Expand All @@ -56,7 +57,7 @@ function useContentBlocks( blockTypes, block ) {
( blockName ) => {
return !! contentBlocksObjectAux[ blockName ];
},
[ blockTypes ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something exhaustive deps linter complained about, and it was right.

[ contentBlocksObjectAux ]
);
return useMemo( () => {
return getContentBlocks( [ block ], isContentBlock );
Expand Down Expand Up @@ -173,19 +174,15 @@ const BlockInspector = ( { showNoBlockSelectedMessage = true } ) => {
const availableTabs = useInspectorControlsTabs( blockType?.name );
const showTabs = availableTabs?.length > 1;

const blockInspectorAnimationSettings = useSelect(
( select ) => {
if ( blockType ) {
const globalBlockInspectorAnimationSettings =
select( blockEditorStore ).getSettings()
.blockInspectorAnimation;
return globalBlockInspectorAnimationSettings?.[
blockType.name
];
}
return null;
},
[ selectedBlockClientId, blockType ]
// The block inspector animation settings will be completely
// removed in the future to create an API which allows the block
// inspector to transition between what it
// displays based on the relationship between the selected block
// and its parent, and only enable it if the parent is controlling
// its children blocks.
const blockInspectorAnimationSettings = useBlockInspectorAnimationSettings(
blockType,
selectedBlockClientId
);

if ( count > 1 ) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';

export default function useBlockInspectorAnimationSettings(
blockType,
selectedBlockClientId
) {
return useSelect(
( select ) => {
if ( blockType ) {
const globalBlockInspectorAnimationSettings =
select( blockEditorStore ).getSettings()
.blockInspectorAnimation;

// Get the name of the block that will allow it's children to be animated.
const animationParent =
globalBlockInspectorAnimationSettings?.animationParent;

// Determine whether the animationParent block is a parent of the selected block.
const { getSelectedBlockClientId, getBlockParentsByBlockName } =
select( blockEditorStore );
const _selectedBlockClientId = getSelectedBlockClientId();
const animationParentBlockClientId = getBlockParentsByBlockName(
_selectedBlockClientId,
animationParent,
true
)[ 0 ];

// If the selected block is not a child of the animationParent block,
// and not an animationParent block itself, don't animate.
if (
! animationParentBlockClientId &&
blockType.name !== animationParent
) {
return null;
}

return globalBlockInspectorAnimationSettings?.[
blockType.name
];
}
return null;
},
[ selectedBlockClientId, blockType ]
);
}
15 changes: 14 additions & 1 deletion packages/block-editor/src/store/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,24 @@ export const SETTINGS_DEFAULTS = {
__unstableGalleryWithImageBlocks: false,
__unstableIsPreviewMode: false,

// This setting is `private` now with `lock` API.
// These settings will be completely revamped in the future.
// The goal is to evolve this into an API which will instruct
// the block inspector to animate transitions between what it
// displays based on the relationship between the selected block
// and its parent, and only enable it if the parent is controlling
// its children blocks.
blockInspectorAnimation: {
animationParent: 'core/navigation',
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't dug deep into blockInspectorAnimation API. Is it an API that we will reuse in other places and blocks? If yes, animationParent(or whatever the final API will be), should be inside each block, like enterDirection is. If not, maybe it's something to be implemented differently and more close to Navigation block? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

It's only used for the navigation block at the moment. I am aware that this implementation won't scale well, but since it's locked I wasn't too worried about that. We can improve it when we need it to do more.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this API is tightly coupled with Navigation block, have you explored a way to handle this 'closer' to the block? Also you have explored and still think it should be here, should we rename to reflect that? blockInspectorAnimation is definitely a generic name, that may create confusion I think..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This APi should work for all blocks that have children which are controlled by the parent in various ways. For now the two examples are navigation block and the pattern block in content only mode.

It is not an API specific to the navigation block and it should not be at the block level.

@ntsekouras which part do you think should be extracted to a hook?

'core/navigation': { enterDirection: 'leftToRight' },
'core/navigation-submenu': { enterDirection: 'rightToLeft' },
'core/navigation-link': { enterDirection: 'rightToLeft' },
'core/search': { enterDirection: 'rightToLeft' },
'core/social-links': { enterDirection: 'rightToLeft' },
'core/page-list': { enterDirection: 'rightToLeft' },
'core/spacer': { enterDirection: 'rightToLeft' },
'core/home-link': { enterDirection: 'rightToLeft' },
'core/site-title': { enterDirection: 'rightToLeft' },
'core/site-logo': { enterDirection: 'rightToLeft' },
},

generateAnchors: false,
Expand Down