Skip to content
Merged
12 changes: 2 additions & 10 deletions packages/ui/src/stepper/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,14 @@ export const StepperRoot = forwardRef< StepperRef, StepperRootProps >( function
},
} ) );

// Dev warnings: missing accessibility label, duplicate values, missing step
// Dev warnings: missing accessibility label, missing step.
// Duplicate step values are detected at registration time in useStepRegistration.
useEffect( () => {
if ( ! ariaLabel && ! ariaLabelledBy ) {
warning(
"[Stepper] Stepper requires either 'aria-label' or 'aria-labelledby' for accessibility."
);
}
const seenValues = new Set< string >();
for ( const s of steps ) {
if ( seenValues.has( s.value ) ) {
warning(
`[Stepper] Two steps share value '${ s.value }'. Each step must have a unique value.`
);
}
seenValues.add( s.value );
}
if ( value && steps.length > 0 && ! steps.some( ( s ) => s.value === value ) ) {
warning(
`[Stepper] No step found with value '${ value }'. Check that the value matches one of the registered step values.`
Expand Down
61 changes: 55 additions & 6 deletions packages/ui/src/stepper/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,61 @@ describe( 'Stepper dev warnings', () => {
spy.mockRestore();
} );

// NOTE: A test for the duplicate-value warning ("Two steps share value '...'") is
// deliberately omitted. useStepRegistration prevents duplicate entries at registration
// time (it bails out if a step with the same value is already in state), so the
// steps array that the useEffect in root.tsx inspects will never contain duplicates.
// The warning is unreachable in a JSDOM environment via normal JSX rendering, making
// any such test a never-fails assertion that proves nothing about real behaviour.
// NOTE: warning() dedupes messages process-wide, so these tests use step values
// unique to this file to make sure the expected message has not fired elsewhere.
it( 'warns when two steps share the same value', async () => {
const spy = jest.spyOn( console, 'warn' ).mockImplementation( () => {} );
render(
<Stepper.Root orientation="vertical" value="dup-warn" aria-label="Test">
<Stepper.Step value="dup-warn">
<Stepper.Indicator />
</Stepper.Step>
<Stepper.Step value="dup-warn">
<Stepper.Indicator />
</Stepper.Step>
</Stepper.Root>
);
await waitFor( () => {
expect( spy ).toHaveBeenCalledWith(
expect.stringContaining( "Two steps share value 'dup-warn'" )
);
} );
spy.mockRestore();
} );

it( 'keeps the original step registered when a duplicate step unmounts', async () => {
const spy = jest.spyOn( console, 'warn' ).mockImplementation( () => {} );
function Dynamic() {
const [ showDuplicate, setShowDuplicate ] = useState( true );
return (
Comment thread
bogiii marked this conversation as resolved.
Resolved
<>
<button onClick={ () => setShowDuplicate( false ) }>Remove duplicate</button>
<Stepper.Root orientation="vertical" value="dup-unmount" aria-label="Test">
<Stepper.Step value="dup-unmount">
<Stepper.Indicator />
</Stepper.Step>
{ showDuplicate && (
<Stepper.Step value="dup-unmount">
<Stepper.Indicator />
</Stepper.Step>
) }
<Stepper.Step value="last">
<Stepper.Indicator />
</Stepper.Step>
</Stepper.Root>
</>
);
}
const user = userEvent.setup();
render( <Dynamic /> );
// The duplicate is rejected at registration time, so only two steps count.
await waitFor( () => expect( screen.getByText( 'Step 2 of 2' ) ).toBeInTheDocument() );
await user.click( screen.getByRole( 'button', { name: /remove duplicate/i } ) );
// Unmounting the rejected duplicate must not deregister the original step.
await waitFor( () => expect( screen.getByText( 'Step 2 of 2' ) ).toBeInTheDocument() );
expect( screen.getByText( 'Step 1 of 2' ) ).toBeInTheDocument();
spy.mockRestore();
} );
} );

describe( 'Stepper.Panel dev warnings', () => {
Expand Down
24 changes: 16 additions & 8 deletions packages/ui/src/stepper/use-step-registration.ts
Comment thread
ciampo marked this conversation as resolved.
Resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useCallback, useState } from '@wordpress/element';
import { useCallback, useRef, useState } from '@wordpress/element';
import { warning } from '../utils/warning';

// ---------------------------------------------------------------------------
// Helpers
Expand All @@ -14,16 +15,23 @@ import { useCallback, useState } from '@wordpress/element';
export function useStepRegistration< T extends { value: string } >() {
const [ steps, setSteps ] = useState< T[] >( [] );

// Tracks accepted registrations so duplicates can be rejected (with a dev
// warning) without putting side effects inside the state updater.
const registeredValues = useRef( new Set< string >() );

const registerStep = useCallback( ( meta: T ) => {
setSteps( ( prev ) => {
// Avoid duplicate registration (e.g. from concurrent remounts or late renders)
if ( prev.some( ( s ) => s.value === meta.value ) ) {
return prev;
}
return [ ...prev, meta ];
} );
if ( registeredValues.current.has( meta.value ) ) {
warning(
`[Stepper] Two steps share value '${ meta.value }'. Each step must have a unique value.`
);
// Rejected duplicate: deregistering it must not affect the original.
Comment thread
bogiii marked this conversation as resolved.
Resolved
return () => {};
}
registeredValues.current.add( meta.value );
setSteps( ( prev ) => [ ...prev, meta ] );

return () => {
registeredValues.current.delete( meta.value );
setSteps( ( prev ) => prev.filter( ( s ) => s.value !== meta.value ) );
};
}, [] );
Expand Down
Loading