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

[milindmore22]

Summary

This pull request updates the carousel block's "Use as Tabs" toggle logic and related tests to improve navigation block handling and ensure correct attribute updates. The main changes include more robust removal and insertion of navigation blocks when toggling the "Use as Tabs" option, setting the correct transition type, and updating test cases to reflect the new logic.

Enhancements to "Use as Tabs" toggle logic:

• When enabling "Use as Tabs", the transition attribute is now set to 'slide' in addition to useTabs: true, ensuring consistent behavior. [1] [2] [3]
• Enabling "Use as Tabs" removes all navigation group containers (core/group containing navigation blocks) and any standalone navigation blocks (carousel-controls, carousel-counter, carousel-dots). [1] [2] [3]
• Disabling "Use as Tabs" removes all tab list blocks and, if no navigation blocks exist, re-inserts a full navigation group container. If some navigation blocks are missing, only the missing ones are re-inserted. [1] [2]

Test updates:

• Test cases have been updated and expanded to verify the correct removal and insertion of navigation and tab list blocks, and to check that the correct attributes are set when toggling "Use as Tabs" on and off. [1] [2] [3]

UI adjustments:

• The "Use as Tabs" toggle is now always displayed in the inspector controls, and the "Transition" and "Loop" controls are only shown when "Use as Tabs" is disabled.

Type of change

• Bug fix
• New feature
• Enhancement/refactor
• Documentation update
• Test update
• Build/CI/tooling

Related issue(s)

Closes #174

What changed

• Fixes issue with carousel transition, remove transition control when tab mode is enabled.
• Switch to default slide transition when tab mode is enabled.
• Remove control group when tab mode is enabled.

Breaking changes

Does this introduce a breaking change? If yes, describe the impact and migration path below.

• Yes — migration path:
• No

Testing

Describe how this was tested.

• Unit tests
• Manual testing
• Cross-browser testing (if UI changes)

Test details:

Screenshots / recordings

If applicable, add screenshots or short recordings.

Checklist

• I have self-reviewed this PR
• I have added/updated tests where needed
• I have updated docs where needed
• I have checked for breaking changes

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors the carousel block’s “Use as Tabs” toggle behavior to better synchronize attributes and to remove/re-add navigation blocks appropriately when switching between tab mode and carousel mode.

Changes:

• When enabling “Use as Tabs”, forces transition: 'slide' and removes navigation block(s)/containers.
• When disabling “Use as Tabs”, removes tab-list blocks and re-inserts missing navigation blocks (or the full nav group) based on what exists.
• Updates/expands Jest tests to assert new attribute updates and block removal/insertion behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
src/blocks/carousel/edit.tsx	Updates toggle logic for tab mode, including attribute resets and navigation/tab-list block removal/insertion; updates Inspector UI visibility.
src/blocks/carousel/tests/edit.test.tsx	Adjusts tests to validate new transition behavior and nav/tab-list block add/remove logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/blocks/carousel/edit.tsx:386

• idsToRemove can include both a core/group nav container and its descendant nav block clientIds. Removing both parent and child IDs in one removeBlocks call can lead to “block not found” behavior (depending on editor internals) and makes the operation order-dependent. Prefer removing only the highest-level containers when present, and only remove standalone nav blocks that are not descendants of a container you’re already removing (and update the corresponding tests to match the intended removal behavior).

			// 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' ),
			];

			const idsToRemove = Array.from(
				new Set( [
					...navGroupBlocks.map( ( b ) => b.clientId ),
					...looseNavBlocks.map( ( b ) => b.clientId ),
				] ),
			);
			if ( idsToRemove.length > 0 ) {
				removeBlocks( idsToRemove );
			}

src/blocks/carousel/edit.tsx:371

• This only detects nav core/group containers that are direct children of the carousel. If a nav group is nested deeper (e.g., within another group/columns), enabling “Use as Tabs” will remove the nav blocks (via findAllBlocksDeep) but can leave behind empty wrapper groups, which contradicts the stated goal of removing “navigation group containers”. Consider collecting nav-group containers via a deep traversal (e.g., “all core/group blocks deep where innerBlocks contain nav blocks”) so nested nav groups are removed too.

			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' ) ),
			);

src/blocks/carousel/edit.tsx:433

• When restoring missing nav blocks, all insertions use index = undefined (append). This can produce an unexpected nav order (e.g., if only dots exist, you end up with [dots, controls, counter]), which can break layout/UX if order matters. Consider inserting at stable positions (controls=0, counter=1, dots=2) relative to existing children within the target parent so the group consistently matches the structure produced by createNavGroup().

			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;

				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,
					);
				}
			}

src/blocks/carousel/tests/edit.test.tsx:517

• This test locks in the current behavior of removing both the nav container (group-1) and its descendant nav blocks in the same call. If you change the production logic to remove only the container (to avoid parent+child removal issues), this test should be updated to assert the intended removal strategy (e.g., only group-1 is removed). Additionally, there’s no coverage for the nested-nav-group case (nav group inside another wrapper), which is where the current implementation can leave empty groups behind.

	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-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',
			] ),
		);
	} );

[danish17]

LGTM.