Skip to content

Fixes : Improved tabs block by removing carousel control group and improve block filter logic #175

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

Merged
milindmore22 merged 4 commits into from
Jul 30, 2026
Merged

Fixes : Improved tabs block by removing carousel control group and improve block filter logic #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87f475b
refactor: automate management of carousel navigation and tab blocks w…
milindmore22 Jul 30, 2026
113e59f
fix: add null coalescing operators to innerBlocks in block filter logic
milindmore22 Jul 30, 2026
5dac541
fix: disable autoScroll when using tabs and improve navigation block …
milindmore22 Jul 30, 2026
050a1e2
refactor: update nav block insertion logic to append at default index…
milindmore22 Jul 30, 2026
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
142 changes: 136 additions & 6 deletions src/blocks/carousel/__tests__/edit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,9 @@ describe( 'useTabs toggle', () => {

toggleCall[ 0 ].onChange( true );

expect( setAttributes ).toHaveBeenCalledWith( expect.objectContaining( { useTabs: true } ) );
expect( setAttributes ).toHaveBeenCalledWith(
expect.objectContaining( { useTabs: true, transition: 'slide', autoScroll: false } ),
);
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-tab-list' } ),
0,
Expand Down Expand Up @@ -436,18 +438,92 @@ describe( 'useTabs toggle', () => {

toggleCall[ 0 ].onChange( true );

expect( setAttributes ).toHaveBeenCalledWith( expect.objectContaining( { useTabs: true } ) );
expect( setAttributes ).toHaveBeenCalledWith(
expect.objectContaining( { useTabs: true, transition: 'slide', autoScroll: false } ),
);
expect( mockInsertBlock ).not.toHaveBeenCalled();
} );

it( 'removes all tab list blocks (including nested ones) when toggling Use as Tabs OFF', () => {
it( 'removes tab list blocks and re-inserts full nav group container when toggling Use as Tabs OFF and no nav blocks exist', () => {
mockBlocks = [
{ name: 'rt-carousel/carousel-tab-list', clientId: 'top-tab-list', innerBlocks: [] },
{ name: 'rt-carousel/carousel-viewport', clientId: 'viewport-1', innerBlocks: [] },
];
const setAttributes = jest.fn();

render(
<Edit
attributes={ { ...createAttributes(), useTabs: true } }
setAttributes={ setAttributes }
clientId="test-client-id"
/>,
);

const toggleCall = ( ToggleControl as unknown as jest.Mock ).mock.calls.find(
( [ props ] ) => props.label === 'Use as Tabs',
);

toggleCall[ 0 ].onChange( false );

expect( setAttributes ).toHaveBeenCalledWith( { useTabs: false } );
expect( mockRemoveBlocks ).toHaveBeenCalledWith( [ 'top-tab-list' ] );
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'core/group' } ),
undefined,
'test-client-id',
);
} );

it( 'removes navigation group row containers and navigation blocks when toggling Use as Tabs ON', () => {
mockBlocks = [
{ name: 'rt-carousel/carousel-viewport', clientId: 'viewport-1', innerBlocks: [] },
{
name: 'core/group',
clientId: 'group-1',
innerBlocks: [
{ name: 'rt-carousel/carousel-tab-list', clientId: 'nested-tab-list', innerBlocks: [] },
{ name: 'rt-carousel/carousel-controls', clientId: 'nested-controls', innerBlocks: [] },
{ name: 'rt-carousel/carousel-counter', clientId: 'nested-counter', innerBlocks: [] },
{ name: 'rt-carousel/carousel-dots', clientId: 'nested-dots', innerBlocks: [] },
],
},
];
const setAttributes = jest.fn();

render(
<Edit
attributes={ createAttributes() }
setAttributes={ setAttributes }
clientId="test-client-id"
/>,
);

const toggleCall = ( ToggleControl as unknown as jest.Mock ).mock.calls.find(
( [ props ] ) => props.label === 'Use as Tabs',
);

toggleCall[ 0 ].onChange( true );

expect( setAttributes ).toHaveBeenCalledWith( expect.objectContaining( { useTabs: true } ) );
const removedIds = mockRemoveBlocks.mock.calls[ 0 ][ 0 ];
expect( removedIds ).toHaveLength( 4 );
expect( removedIds ).toEqual(
expect.arrayContaining( [
'group-1',
'nested-controls',
'nested-counter',
'nested-dots',
] ),
);
} );

it( 'restores missing nav blocks into the core/group that actually contains nav elements, ignoring unrelated groups', () => {
mockBlocks = [
{ name: 'core/group', clientId: 'unrelated-group', innerBlocks: [] },
{
name: 'core/group',
clientId: 'nav-group-1',
innerBlocks: [
{ name: 'rt-carousel/carousel-dots', clientId: 'existing-dots', innerBlocks: [] },
],
},
];
Expand All @@ -467,7 +543,61 @@ describe( 'useTabs toggle', () => {

toggleCall[ 0 ].onChange( false );

expect( setAttributes ).toHaveBeenCalledWith( { useTabs: false } );
expect( mockRemoveBlocks ).toHaveBeenCalledWith( [ 'top-tab-list', 'nested-tab-list' ] );
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-controls' } ),
undefined,
'nav-group-1',
);
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-counter' } ),
undefined,
'nav-group-1',
);
} );

it( 're-inserts only missing nav blocks (counter and dots) when controls already exist in a nav group', () => {
mockBlocks = [
{
name: 'core/group',
clientId: 'nav-group-1',
innerBlocks: [
{ name: 'rt-carousel/carousel-controls', clientId: 'existing-controls', innerBlocks: [] },
],
},
];
const setAttributes = jest.fn();

render(
<Edit
attributes={ { ...createAttributes(), useTabs: true } }
setAttributes={ setAttributes }
clientId="test-client-id"
/>,
);

const toggleCall = ( ToggleControl as unknown as jest.Mock ).mock.calls.find(
( [ props ] ) => props.label === 'Use as Tabs',
);

toggleCall[ 0 ].onChange( false );

// Controls already exist — should NOT be inserted again
expect( mockInsertBlock ).not.toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-controls' } ),
expect.anything(),
expect.anything(),
);

// Missing counter and dots SHOULD be inserted into nav-group-1
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-counter' } ),
undefined,
'nav-group-1',
);
expect( mockInsertBlock ).toHaveBeenCalledWith(
expect.objectContaining( { name: 'rt-carousel/carousel-dots' } ),
undefined,
'nav-group-1',
);
} );
} );
105 changes: 85 additions & 20 deletions src/blocks/carousel/edit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,14 @@ export default function Edit( {
if ( value ) {
setAttributes( {
useTabs: true,
transition: 'slide',
loop: false,
dragFree: false,
carouselAlign: 'start',
Comment thread
milindmore22 marked this conversation as resolved.
containScroll: 'trimSnaps',
slidesToScroll: '1',
autoplay: false,
autoScroll: false,
axis: 'x',
} );
const tabListExists = findBlockDeep( innerBlocks, 'rt-carousel/carousel-tab-list' );
Expand All @@ -359,40 +361,83 @@ export default function Edit( {
clientId,
);
}
// Find nav row containers (core/group containing navigation blocks) as well as any standalone nav blocks
const navGroupBlocks = innerBlocks.filter(
( b ) =>
b.name === 'core/group' &&
( findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-controls' ) ||
findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-counter' ) ||
findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-dots' ) ),
);
const looseNavBlocks = [
...findAllBlocksDeep( innerBlocks, 'rt-carousel/carousel-controls' ),
...findAllBlocksDeep( innerBlocks, 'rt-carousel/carousel-counter' ),
...findAllBlocksDeep( innerBlocks, 'rt-carousel/carousel-dots' ),
];
Comment thread
milindmore22 marked this conversation as resolved.

const idsToRemove = Array.from(
new Set( [
...navGroupBlocks.map( ( b ) => b.clientId ),
...looseNavBlocks.map( ( b ) => b.clientId ),
] ),
);
if ( idsToRemove.length > 0 ) {
removeBlocks( idsToRemove );
}
} else {
setAttributes( { useTabs: false } );
const tabListBlocks = findAllBlocksDeep( innerBlocks, 'rt-carousel/carousel-tab-list' );
const tabListIds = tabListBlocks.map( ( b ) => b.clientId );
if ( tabListIds.length > 0 ) {
removeBlocks( tabListIds );
}

const controlsExist = findBlockDeep( innerBlocks, 'rt-carousel/carousel-controls' );
const counterExist = findBlockDeep( innerBlocks, 'rt-carousel/carousel-counter' );
const dotsExist = findBlockDeep( innerBlocks, 'rt-carousel/carousel-dots' );

if ( ! controlsExist && ! counterExist && ! dotsExist ) {
// No navigation blocks exist — insert full nav group row container
insertBlock( createNavGroup(), undefined, clientId );
} else if ( ! controlsExist || ! counterExist || ! dotsExist ) {
const navGroup = innerBlocks.find(
( b ) =>
b.name === 'core/group' &&
( findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-controls' ) ||
findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-counter' ) ||
findBlockDeep( b.innerBlocks ?? [], 'rt-carousel/carousel-dots' ) ),
);
const targetParentId = navGroup ? navGroup.clientId : clientId;
Comment thread
milindmore22 marked this conversation as resolved.
Comment thread
milindmore22 marked this conversation as resolved.

if ( ! controlsExist ) {
insertBlock(
createBlock( 'rt-carousel/carousel-controls', {} ),
undefined,
targetParentId,
);
}
if ( ! counterExist ) {
insertBlock(
createBlock( 'rt-carousel/carousel-counter', {} ),
undefined,
targetParentId,
);
}
if ( ! dotsExist ) {
insertBlock(
createBlock( 'rt-carousel/carousel-dots', {} ),
undefined,
targetParentId,
);
}
Comment thread
milindmore22 marked this conversation as resolved.
}
}
};

const inspectorControls = (
<>
<InspectorControls>
<PanelBody title={ useTabs ? __( 'Tab Settings', 'rt-carousel' ) : __( 'Carousel Settings', 'rt-carousel' ) }>
<SelectControl
label={ __( 'Transition', 'rt-carousel' ) }
value={ transition }
disabled={ autoScroll }
options={ [
{ label: __( 'Slide', 'rt-carousel' ), value: 'slide' },
{ label: __( 'Fade', 'rt-carousel' ), value: 'fade' },
] }
onChange={ ( value ) =>
setAttributes( { transition: value as CarouselAttributes[ 'transition' ] } )
}
help={
autoScroll
? __( 'Auto Scroll does not support transitions.', 'rt-carousel' )
: __(
'Choose how slides transition: sliding horizontally or cross-fading.',
'rt-carousel',
)
}
/>
<ToggleControl
label={ __( 'Use as Tabs', 'rt-carousel' ) }
checked={ useTabs }
Expand All @@ -404,6 +449,26 @@ export default function Edit( {
/>
{ ! useTabs && (
<>
<SelectControl
label={ __( 'Transition', 'rt-carousel' ) }
value={ transition }
disabled={ autoScroll }
options={ [
{ label: __( 'Slide', 'rt-carousel' ), value: 'slide' },
{ label: __( 'Fade', 'rt-carousel' ), value: 'fade' },
] }
onChange={ ( value ) =>
setAttributes( { transition: value as CarouselAttributes[ 'transition' ] } )
}
help={
autoScroll
? __( 'Auto Scroll does not support transitions.', 'rt-carousel' )
: __(
'Choose how slides transition: sliding horizontally or cross-fading.',
'rt-carousel',
)
}
/>
<ToggleControl
label={ __( 'Loop', 'rt-carousel' ) }
checked={ loop }
Expand Down
Loading