Skip to content

Commit

Permalink
Components: replace TabPanel with Tabs in the Block Inserter (#56918
Browse files Browse the repository at this point in the history
)

* implement `Tabs`

* update styles

* focusable false

* replace render function with object

* fix inserter tests

* pass contents as prop instead of children
  • Loading branch information
chad1008 authored Dec 13, 2023
1 parent 4844fe9 commit 37401d5
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 28 deletions.
23 changes: 9 additions & 14 deletions packages/block-editor/src/components/inserter/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,17 +191,13 @@ function InserterMenu(
]
);

const getCurrentTab = useCallback(
( tab ) => {
if ( tab.name === 'blocks' ) {
return blocksTab;
} else if ( tab.name === 'patterns' ) {
return patternsTab;
} else if ( tab.name === 'media' ) {
return mediaTab;
}
},
[ blocksTab, patternsTab, mediaTab ]
const inserterTabsContents = useMemo(

This comment has been minimized.

Copy link
@ellatrix

ellatrix Dec 13, 2023

Member

Why is this memoized? Is InserterSearchResults a pure/memoized component based on its props?

This comment has been minimized.

Copy link
@ciampo

ciampo Dec 13, 2023

Contributor

InserterSearchResults is not a pure component, I guess @chad1008 memorized it based on the fact that previously the getCurrentTab function was also memorized via useCallback

() => ( {
blocks: blocksTab,
patterns: patternsTab,
media: mediaTab,
} ),
[ blocksTab, mediaTab, patternsTab ]
);

const searchRef = useRef();
Expand Down Expand Up @@ -275,9 +271,8 @@ function InserterMenu(
showMedia={ showMedia }
prioritizePatterns={ prioritizePatterns }
onSelect={ handleSetSelectedTab }
>
{ getCurrentTab }
</InserterTabs>
tabsContents={ inserterTabsContents }
/>
) }
{ ! delayedFilterValue && ! showAsTabs && (
<div className="block-editor-inserter__no-tab-container">
Expand Down
8 changes: 4 additions & 4 deletions packages/block-editor/src/components/inserter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ $block-inserter-tabs-height: 44px;
.block-editor-inserter__popover .block-editor-inserter__menu {
margin: -$grid-unit-15;

.block-editor-inserter__tabs .components-tab-panel__tabs {
.block-editor-inserter__tabs div[role="tablist"] {
top: $grid-unit-10 + $grid-unit-20 + $grid-unit-60 - $grid-unit-15;
}

Expand Down Expand Up @@ -118,10 +118,10 @@ $block-inserter-tabs-height: 44px;
flex-direction: column;
overflow: hidden;

.components-tab-panel__tabs {
div[role="tablist"] {
border-bottom: $border-width solid $gray-300;

.components-tab-panel__tabs-item {
button[role="tab"] {
flex-grow: 1;
margin-bottom: -$border-width;
&[id$="reusable"] {
Expand All @@ -133,7 +133,7 @@ $block-inserter-tabs-height: 44px;
}
}

.components-tab-panel__tab-content {
div[role="tabpanel"] {
display: flex;
flex-grow: 1;
flex-direction: column;
Expand Down
38 changes: 29 additions & 9 deletions packages/block-editor/src/components/inserter/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,16 @@
* WordPress dependencies
*/
import { useMemo } from '@wordpress/element';
import { TabPanel } from '@wordpress/components';
import { privateApis as componentsPrivateApis } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';

const { Tabs } = unlock( componentsPrivateApis );

const blocksTab = {
name: 'blocks',
/* translators: Blocks tab title in the block inserter. */
Expand All @@ -23,11 +30,11 @@ const mediaTab = {
};

function InserterTabs( {
children,
showPatterns = false,
showMedia = false,
onSelect,
prioritizePatterns,
tabsContents,
} ) {
const tabs = useMemo( () => {
const tempTabs = [];
Expand All @@ -45,13 +52,26 @@ function InserterTabs( {
}, [ prioritizePatterns, showPatterns, showMedia ] );

return (
<TabPanel
className="block-editor-inserter__tabs"
tabs={ tabs }
onSelect={ onSelect }
>
{ children }
</TabPanel>
<div className="block-editor-inserter__tabs">
<Tabs onSelect={ onSelect }>
<Tabs.TabList>
{ tabs.map( ( tab ) => (
<Tabs.Tab key={ tab.name } tabId={ tab.name }>
{ tab.title }
</Tabs.Tab>
) ) }
</Tabs.TabList>
{ tabs.map( ( tab ) => (
<Tabs.TabPanel
key={ tab.name }
tabId={ tab.name }
focusable={ false }
>
{ tabsContents[ tab.name ] }
</Tabs.TabPanel>
) ) }
</Tabs>
</div>
);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/e2e-test-utils/src/inserter.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export async function selectGlobalInserterTab( label ) {
}

const activeTab = await page.waitForSelector(
'.block-editor-inserter__tabs button.is-active'
// Targeting a class name is necessary here, because there are likely
// two implementations of the `Tabs` component visible to this test, and
// we want to confirm that it's waiting for the correct one.
'.block-editor-inserter__tabs [role="tab"][aria-selected="true"]'
);

const activeTabLabel = await page.evaluate(
Expand Down

1 comment on commit 37401d5

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 37401d5.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7199491432
📝 Reported issues:

Please sign in to comment.