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

Deprecate the original slot implementation and refactor usage #22027

Closed
wants to merge 3 commits into from
Closed

Deprecate the original slot implementation and refactor usage #22027

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
2d10ca6
Deprecate the original slot implementation and refactor usage
youknowriad May 1, 2020
fad752a
Fix format toolbar
youknowriad May 4, 2020
8b81838
Fix inserter slot
youknowriad May 5, 2020
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: 18 additions & 7 deletions packages/block-editor/src/components/block-settings-menu-controls/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
/**
* External dependencies
*/
import { isEmpty, map } from 'lodash';
import { map } from 'lodash';

/**
* WordPress dependencies
*/
import { createSlotFill, MenuGroup } from '@wordpress/components';
import {
createSlotFill,
MenuGroup,
__experimentalUseSlot as useSlot,
} from '@wordpress/components';
import { useSelect } from '@wordpress/data';

const { Fill: BlockSettingsMenuControls, Slot } = createSlotFill(
Expand All @@ -25,13 +29,20 @@ const BlockSettingsMenuControlsSlot = ( { fillProps } ) => {
),
};
}, [] );
const slot = useSlot( 'BlockSettingsMenuControls' );
const hasFills = Boolean( slot.fills && slot.fills.length );
Copy link
Member

@gziolo gziolo May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It’s used nearly everywhere, how about useSlot exposes hasFills by default?


if ( ! hasFills ) {
return null;
}

return (
<Slot fillProps={ { ...fillProps, selectedBlocks } }>
{ ( fills ) =>
! isEmpty( fills ) && <MenuGroup>{ fills }</MenuGroup>
}
</Slot>
<MenuGroup>
<Slot
bubblesVirtually
fillProps={ { ...fillProps, selectedBlocks } }
/>
</MenuGroup>
);
};

Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/block-settings-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export function BlockSettingsMenu( { clientIds } ) {
<>
<MenuGroup>
<__experimentalBlockSettingsMenuFirstItem.Slot
bubblesVirtually
fillProps={ { onClose } }
/>
{ count === 1 && (
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/inserter-menu-extension/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ const { Fill: __experimentalInserterMenuExtension, Slot } = createSlotFill(
);

__experimentalInserterMenuExtension.Slot = Slot;
__experimentalInserterMenuExtension.Slot.slotName =
'__experimentalInserterMenuExtension';

export default __experimentalInserterMenuExtension;
81 changes: 43 additions & 38 deletions packages/block-editor/src/components/inserter/block-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@ import {
* WordPress dependencies
*/
import { __, _x, _n, sprintf } from '@wordpress/i18n';
import { withSpokenMessages } from '@wordpress/components';
import {
withSpokenMessages,
__experimentalUseSlot as useSlot,
} from '@wordpress/components';
import { addQueryArgs } from '@wordpress/url';
import { controlsRepeat } from '@wordpress/icons';
import { speak } from '@wordpress/a11y';
import { createBlock } from '@wordpress/blocks';
import { useMemo, useEffect } from '@wordpress/element';
import { useMemo, useEffect, useCallback } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { compose } from '@wordpress/compose';

Expand Down Expand Up @@ -97,22 +100,25 @@ function InserterBlockList( {
}
}, [] );

const onSelectItem = ( item ) => {
const { name, title, initialAttributes, innerBlocks } = item;
const insertedBlock = createBlock(
name,
initialAttributes,
createBlocksFromInnerBlocksTemplate( innerBlocks )
);
const onSelectItem = useCallback(
( item ) => {
const { name, title, initialAttributes, innerBlocks } = item;
const insertedBlock = createBlock(
name,
initialAttributes,
createBlocksFromInnerBlocksTemplate( innerBlocks )
);

onInsert( insertedBlock );
onInsert( insertedBlock );

if ( ! selectBlockOnInsert ) {
// translators: %s: the name of the block that has been added
const message = sprintf( __( '%s block added' ), title );
speak( message );
}
};
if ( ! selectBlockOnInsert ) {
// translators: %s: the name of the block that has been added
const message = sprintf( __( '%s block added' ), title );
speak( message );
}
},
[ onInsert, selectBlockOnInsert ]
);

const filteredItems = useMemo( () => {
return searchBlockItems( items, categories, collections, filterValue );
Expand Down Expand Up @@ -187,6 +193,10 @@ function InserterBlockList( {

const hasItems = ! isEmpty( filteredItems );
const hasChildItems = childItems.length > 0;
const slot = useSlot( __experimentalInserterMenuExtension.Slot.slotName );
const hasInserterExtensionFills = Boolean(
slot.fills && slot.fills.length
);

return (
<div>
Expand Down Expand Up @@ -272,28 +282,23 @@ function InserterBlockList( {
</InserterPanel>
) }

<__experimentalInserterMenuExtension.Slot
fillProps={ {
onSelect: onSelectItem,
onHover,
filterValue,
hasItems,
} }
>
{ ( fills ) => {
if ( fills.length ) {
return (
<InserterPanel title={ __( 'Search Results' ) }>
{ fills }
</InserterPanel>
);
}
if ( ! hasItems ) {
return <InserterNoResults />;
}
return null;
} }
</__experimentalInserterMenuExtension.Slot>
{ hasInserterExtensionFills && ! hasItems && (
<InserterPanel title={ __( 'Search Results' ) }>
<__experimentalInserterMenuExtension.Slot
bubblesVirtually
fillProps={ {
onSelect: onSelectItem,
onHover,
filterValue,
hasItems,
} }
/>
</InserterPanel>
) }

{ ! hasInserterExtensionFills && ! hasItems && (
<InserterNoResults />
) }
</div>
);
}
Expand Down
71 changes: 41 additions & 30 deletions packages/block-editor/src/components/inserter/menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { includes, pick } from 'lodash';
/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';
import { useState, useCallback } from '@wordpress/element';
import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes';
import { TabPanel } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -91,7 +91,7 @@ function InserterMenu( {
// Since it's a function only called when the event handlers are called,
// it's fine to extract it.
// eslint-disable-next-line no-restricted-syntax
function getInsertionIndex() {
const getInsertionIndex = useCallback( () => {
// If the clientId is defined, we insert at the position of the block.
if ( clientId ) {
return getBlockIndex( clientId, destinationRootClientId );
Expand All @@ -105,37 +105,48 @@ function InserterMenu( {

// Otherwise, we insert at the end of the current rootClientId
return getBlockOrder( destinationRootClientId ).length;
}
}, [ isAppender, clientId, destinationRootClientId ] );

const onInsertBlocks = ( blocks ) => {
const selectedBlock = getSelectedBlock();
if (
! isAppender &&
selectedBlock &&
isUnmodifiedDefaultBlock( selectedBlock )
) {
replaceBlocks( selectedBlock.clientId, blocks );
} else {
insertBlocks(
blocks,
getInsertionIndex(),
destinationRootClientId,
__experimentalSelectBlockOnInsert
);
}
const onInsertBlocks = useCallback(
( blocks ) => {
const selectedBlock = getSelectedBlock();
if (
! isAppender &&
selectedBlock &&
isUnmodifiedDefaultBlock( selectedBlock )
) {
replaceBlocks( selectedBlock.clientId, blocks );
} else {
insertBlocks(
blocks,
getInsertionIndex(),
destinationRootClientId,
__experimentalSelectBlockOnInsert
);
}

onSelect();
};
onSelect();
},
[
isAppender,
getInsertionIndex,
destinationRootClientId,
__experimentalSelectBlockOnInsert,
]
);

const onHover = ( item ) => {
setHoveredItem( item );
if ( item ) {
const index = getInsertionIndex();
showInsertionPoint( destinationRootClientId, index );
} else {
hideInsertionPoint();
}
};
const onHover = useCallback(
( item ) => {
setHoveredItem( item );
if ( item ) {
const index = getInsertionIndex();
showInsertionPoint( destinationRootClientId, index );
} else {
hideInsertionPoint();
}
},
[ getInsertionIndex, destinationRootClientId ]
);

const blocksTab = (
<>
Expand Down
74 changes: 51 additions & 23 deletions packages/block-editor/src/components/rich-text/format-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
/**
* External dependencies
*/

import { orderBy } from 'lodash';

/**
* WordPress dependencies
*/

import { __ } from '@wordpress/i18n';
import { Toolbar, Slot, DropdownMenu } from '@wordpress/components';
import {
Toolbar,
Slot,
Dropdown,
Button,
NavigableMenu,
__experimentalUseSlot as useSlot,
} from '@wordpress/components';
import { DOWN } from '@wordpress/keycodes';
import { chevronDown } from '@wordpress/icons';

const POPOVER_PROPS = {
Expand All @@ -18,6 +19,9 @@ const POPOVER_PROPS = {
};

const FormatToolbar = () => {
const slot = useSlot( 'RichText.ToolbarControls' );
const hasFills = Boolean( slot.fills && slot.fills.length );

return (
<div className="block-editor-format-toolbar">
<Toolbar>
Expand All @@ -26,24 +30,48 @@ const FormatToolbar = () => {
<Slot
name={ `RichText.ToolbarControls.${ format }` }
key={ format }
bubblesVirtually
/>
)
) }
<Slot name="RichText.ToolbarControls">
{ ( fills ) =>
fills.length !== 0 && (
<DropdownMenu
icon={ chevronDown }
label={ __( 'More rich text controls' ) }
controls={ orderBy(
fills.map( ( [ { props } ] ) => props ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mapping through fills and using them as "controls" is not something I was expecting. I'm not sure how we can solve this here? Why can't we just use a render Slot in renderContent of a "Dropdown" component. Anyway, @ellatrix, maybe you can help solve this. (bubbles virtually don't support children as function and can't support it)

Copy link
Member

Choose a reason for hiding this comment

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

Why can't we just use a render Slot in renderContent of a "Dropdown" component.

Could you elaborate?
I'm also not sure how to fix this here. Normally we use a controls array with control objects, but in the case of RichText, components are used. Not sure if we can change that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a fix, let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

How are they now sorted alphabetically?

Copy link
Member

Choose a reason for hiding this comment

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

I'm also not sure how the collapsed button can now know if any buttons are active. I guess this would be easier with a controls array.

Another thing I'm noticing is that the buttons have too much space in between them compared to master

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are not sorted, I believe they are probably sorted at the "import" level.

The thing is, with Slot/Fill, we're not supposed to know the shape of the children... So I'd say we were doing it wrong previously or Slot/Fill, is not the right API here.

Copy link
Member

Choose a reason for hiding this comment

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

I'll have a look at it now

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I tried converting it to control object but that doesn't really work because we're stuck with the React component rendered in the format type edit functions. Maybe we can create a slot and fill for each button, which could be sorted by the title of the format type?

Copy link
Member

Choose a reason for hiding this comment

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

It still wouldn't work perfectly though, because some format types could render multiple buttons. That's more of an edge case though.

'title'
) }
popoverProps={ POPOVER_PROPS }
/>
)
}
</Slot>
{ hasFills && (
<Dropdown
popoverProps={ POPOVER_PROPS }
renderToggle={ ( { isOpen, onToggle } ) => {
const openOnArrowDown = ( event ) => {
if ( ! isOpen && event.keyCode === DOWN ) {
event.preventDefault();
event.stopPropagation();
onToggle();
}
};

return (
<Button
icon={ chevronDown }
onClick={ onToggle }
onKeyDown={ openOnArrowDown }
aria-haspopup="true"
aria-expanded={ isOpen }
label={ __( 'More rich text controls' ) }
showTooltip
/>
);
} }
renderContent={ ( { onClose } ) => (
<NavigableMenu
aria-label={ __( 'More rich text controls' ) }
role="menu"
>
<Slot
name="RichText.ToolbarControls"
bubblesVirtually
fillProps={ { onClose } }
/>
</NavigableMenu>
) }
/>
) }
</Toolbar>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/rich-text/format-toolbar/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const FormatToolbar = () => {
<Slot
name={ `RichText.ToolbarControls.${ format }` }
key={ format }
bubblesVirtually
/>
) ) }
<Slot name="RichText.ToolbarControls" />
Expand Down
6 changes: 6 additions & 0 deletions packages/block-editor/src/components/rich-text/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,9 @@ figcaption.block-editor-rich-text__editable [data-rich-text-placeholder]::before
padding-right: $grid-unit-15;
}
}


.block-editor-rich-text__advanced-toolbar-button {
display: flex;
width: 100%;
}
Loading