Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- `Menu`: Return focus to the root trigger after closing a legacy `Modal` opened from a menu item, while preserving the menu-to-Modal scroll-lock handoff ([#81164](https://github.com/WordPress/gutenberg/pull/81164)).
- `Button`: Suppress the browser focus ring when keyboard-focused and pressed ([#81113](https://github.com/WordPress/gutenberg/pull/81113)).
- `InputControl`: Vertically center the value of date and time inputs in Safari ([#81361](https://github.com/WordPress/gutenberg/pull/81361)).
- `ControlWithError`: Re-read the target's validation message when an `invalid` event is received, so a message that changed without a re-render in between (e.g. a programmatic value change followed by a synthetic `invalid` event) is not revealed stale or empty. While a `validating` custom validity is pending, the message is left untouched so the pending indicator keeps showing ([#81440](https://github.com/WordPress/gutenberg/pull/81440)).

### TypeScript

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,20 @@ function UnforwardedControlWithError< C extends React.ReactElement >(
useEffect( () => {
const validityTarget = getValidityTarget();
const handler = () => {
// Re-read the message: the target's validity may have changed
// since it was last sampled, without a re-render in between.
// While async validation is pending, keep its indicator instead
// of showing a message its result may supersede.
if ( customValidity?.type !== 'validating' ) {
setErrorMessage( validityTarget?.validationMessage );
}
Comment thread
ntsekouras marked this conversation as resolved.
setShowMessage( true );
validityTarget?.setAttribute( VALIDITY_VISIBLE_ATTRIBUTE, '' );
};

validityTarget?.addEventListener( 'invalid', handler );
return () => validityTarget?.removeEventListener( 'invalid', handler );
}, [ getValidityTarget ] );
}, [ customValidity?.type, getValidityTarget ] );

// Suppress the native error popover, while keeping the focus behavior intact.
useEffect( () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,56 @@ describe( 'ControlWithError', () => {
} );
} );

describe( 'Reveal during pending validation', () => {
it( 'should keep the pending indicator instead of a native error on a synthetic `invalid` event', async () => {
const user = userEvent.setup();

function PendingValidatedInputControl() {
const ref = useRef< HTMLInputElement >( null );
return (
<>
<ValidatedInputControl
ref={ ref }
label="Text"
required
value=""
onChange={ () => {} }
customValidity={ {
type: 'validating',
message: 'Validating...',
} }
/>
<button
type="button"
onClick={ () =>
ref.current?.dispatchEvent(
new Event( 'invalid', {
cancelable: true,
} )
)
}
>
Show errors
</button>
</>
);
}

render( <PendingValidatedInputControl /> );

await user.click(
screen.getByRole( 'button', { name: 'Show errors' } )
);

await waitFor( () => {
expect( screen.getByText( 'Validating...' ) ).toBeVisible();
} );
expect(
screen.queryByText( 'Constraints not satisfied' )
).not.toBeInTheDocument();
} );
} );

describe( 'Form submission', () => {
const CustomValidatedInputControl = ( {
...restProps
Expand Down
1 change: 1 addition & 0 deletions packages/dataviews/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

### Bug Fix

- DataForm: Send a single update per calendar interaction in the `datetime` control instead of two identical ones. Selecting or clearing a date now reveals the validation message by firing a synthetic `invalid` event on the input, rather than briefly moving focus into it and re-sending the value, and announces it to screen readers since focus stays on the calendar. [#81440](https://github.com/WordPress/gutenberg/pull/81440)
- DataForms: Skip validation for fields hidden through the `isVisible` API, so a hidden field with validation rules (e.g. `required`) no longer makes the form invalid. Toggling a field's visibility now clears or restores its validity accordingly. [#81377](https://github.com/WordPress/gutenberg/pull/81377)
- DataViews: Pass only the eligible items to a bulk action's `callback`. A bulk action is offered when any one selected item is eligible for it, so the callback could run against items it had declared, through `isEligible`, that it could not handle. [#81198](https://github.com/WordPress/gutenberg/pull/81198)
- DataViews: Fix the `between` date filter discarding a manually entered `From`/`To` date on blur. The control now commits an incomplete range with an unfilled bound — which neither filters nor renders a chip — instead of waiting for both dates, so a typed date survives tabbing away and a range can be entered manually at all. [#81150](https://github.com/WordPress/gutenberg/pull/81150)
Expand Down
56 changes: 24 additions & 32 deletions packages/dataviews/src/components/dataform-controls/datetime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
} from '@wordpress/components';
import { useCallback, useEffect, useRef, useState } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { speak } from '@wordpress/a11y';
import { dateI18n, getDate, getSettings } from '@wordpress/date';
import { Calendar, Stack } from '@wordpress/ui';
import type { DataFormControlProps, FormatDatetime } from '../../types';
Expand Down Expand Up @@ -47,7 +48,6 @@ function CalendarDateTimeControl< Item >( {
const inputControlRef = useRef< HTMLInputElement >( null );
const validationTimeoutRef =
useRef< ReturnType< typeof setTimeout > >( undefined );
const previousFocusRef = useRef< Element | null >( null );

const { minConstraint, maxConstraint, disabledMatchers } =
useDisabledDateMatchers( isValid, parseDateTime );
Expand All @@ -60,16 +60,11 @@ function CalendarDateTimeControl< Item >( {

// Cleanup timeout on unmount
useEffect( () => {
return () => {
if ( validationTimeoutRef.current ) {
clearTimeout( validationTimeoutRef.current );
}
};
return () => clearTimeout( validationTimeoutRef.current );
}, [] );

const onSelectDate = useCallback(
( newDate: Date | null ) => {
let dateTimeValue: string | undefined;
if ( newDate ) {
// Extract the date part in WP timezone from the calendar selection
const wpDate = dateI18n( 'Y-m-d', newDate );
Expand All @@ -84,36 +79,33 @@ function CalendarDateTimeControl< Item >( {

// Combine date and time in WP timezone and convert to ISO
const finalDateTime = getDate( `${ wpDate }T${ wpTime }` );
dateTimeValue = finalDateTime.toISOString();
onChangeCallback( dateTimeValue );

// Clear any existing timeout
if ( validationTimeoutRef.current ) {
clearTimeout( validationTimeoutRef.current );
}
onChangeCallback( finalDateTime.toISOString() );
} else {
onChangeCallback( undefined );
}
// Save the currently focused element
previousFocusRef.current =
inputControlRef.current &&
inputControlRef.current.ownerDocument.activeElement;

// Trigger validation display by simulating focus, blur, and changes.
// Use a timeout to ensure it runs after the value update.
// A calendar interaction counts as touching the field: reveal the
// input's validity state by firing a synthetic `invalid` event,
// which the validated control listens to in order to display its
// error message without moving focus (unlike `reportValidity()`).
// The control re-reads the message on this event, so dispatching
// unconditionally is also what clears a stale error once a valid
// date is selected.
// The timeout ensures the input has re-rendered with the new
// value before its validity is sampled.
clearTimeout( validationTimeoutRef.current );
validationTimeoutRef.current = setTimeout( () => {
if ( inputControlRef.current ) {
inputControlRef.current.focus();
inputControlRef.current.blur();
onChangeCallback( dateTimeValue );

// Restore focus to the previously focused element
if (
previousFocusRef.current &&
previousFocusRef.current instanceof HTMLElement
) {
previousFocusRef.current.focus();
}
const input = inputControlRef.current;
if ( ! input ) {
return;
}
input.dispatchEvent(
new Event( 'invalid', { cancelable: true } )
);
Comment thread
ntsekouras marked this conversation as resolved.
// Focus stays on the calendar, so announce the message;
// revealing it alone would go unnoticed by screen readers.
if ( input.validationMessage ) {
speak( input.validationMessage );
}
}, 0 );
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,20 @@ function UnforwardedControlWithError< C extends React.ReactElement >(
useEffect( () => {
const validityTarget = getValidityTarget();
const handler = () => {
// Re-read the message: the target's validity may have changed
// since it was last sampled, without a re-render in between.
// While async validation is pending, keep its indicator instead
// of showing a message its result may supersede.
if ( customValidity?.type !== 'validating' ) {
setErrorMessage( validityTarget?.validationMessage );
}
setShowMessage( true );
validityTarget?.setAttribute( VALIDITY_VISIBLE_ATTRIBUTE, '' );
};

validityTarget?.addEventListener( 'invalid', handler );
return () => validityTarget?.removeEventListener( 'invalid', handler );
}, [ getValidityTarget ] );
}, [ customValidity?.type, getValidityTarget ] );

// Suppress the native error popover, while keeping the focus behavior intact.
useEffect( () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { useRef } from '@wordpress/element';
import { ValidatedSelectControl } from '../select-control';

describe( 'ControlWithError', () => {
describe( 'Reveal during pending validation', () => {
it( 'should keep the pending indicator instead of a native error on a synthetic `invalid` event', async () => {
const user = userEvent.setup();

function PendingValidatedSelectControl() {
const ref = useRef< HTMLSelectElement >( null );
return (
<>
<ValidatedSelectControl
ref={ ref }
label="Color"
required
value=""
options={ [
{ label: 'Select a color...', value: '' },
{ label: 'Red', value: 'red' },
] }
onChange={ () => {} }
customValidity={ {
type: 'validating',
message: 'Validating...',
} }
/>
<button
type="button"
onClick={ () =>
ref.current?.dispatchEvent(
new Event( 'invalid', {
cancelable: true,
} )
)
}
>
Show errors
</button>
</>
);
}

render( <PendingValidatedSelectControl /> );

await user.click(
screen.getByRole( 'button', { name: 'Show errors' } )
);

await waitFor( () => {
expect( screen.getByText( 'Validating...' ) ).toBeVisible();
} );
expect(
screen.queryByText( 'Constraints not satisfied' )
).not.toBeInTheDocument();
} );
} );
} );
119 changes: 119 additions & 0 deletions packages/dataviews/src/dataform/test/dataform.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1000,4 +1000,123 @@ describe( 'DataForm component', () => {
expect( speak ).not.toHaveBeenCalled();
} );
} );

describe( 'datetime fields', () => {
const datetimeFields = [
{
id: 'date',
label: 'Date',
type: 'datetime' as const,
isValid: { required: true },
},
];

const datetimeForm = {
fields: [ 'date' ],
};

const dayButton = ( date: Date ) =>
screen.getByRole( 'button', {
name: new RegExp(
new Intl.DateTimeFormat( 'en-US', {
weekday: 'long',
year: 'numeric',
month: 'long',
day: 'numeric',
} ).format( date )
),
} );

// Waits out any timeouts scheduled by the control, so that a
// duplicate update scheduled for a later tick is not missed.
const flushTimeouts = () =>
act(
() => new Promise( ( resolve ) => setTimeout( resolve, 10 ) )

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: the control only schedules 0 ms timeouts, so I guess the 10 ms sleep is arbitrary. Could we use 0 ms here (or fake timers) so the test does not rely on a magic number? Harmless as-is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think 10 is fine (upper bound) because while we could use 0 ms to catch duplicates at the same tick, the slightly bigger timeout could catch cases with a slightly later timeout. I agree 10 is still arbitrary and could update if you want. I don't have strong opinions on this 😄

);

function ControlledForm( {
onChange: onChangeProp = noop,
}: {
onChange?: ( edits: { date?: string } ) => void;
} ) {
const [ item, setItem ] = useState< {
date: string | undefined;
} >( { date: '2026-01-10T10:00:00.000Z' } );
return (
<Dataform
onChange={ ( edits ) => {
onChangeProp( edits );
setItem( ( prev ) => ( { ...prev, ...edits } ) );
} }
fields={ datetimeFields }
form={ datetimeForm }
data={ item }
/>
);
}

it( 'should call onChange once when a date is selected in the calendar', async () => {
const onChange = jest.fn();
const user = userEvent.setup();
render(
<Dataform
onChange={ onChange }
fields={ datetimeFields }
form={ datetimeForm }
data={ { date: '2026-01-10T10:00:00.000Z' } }
/>
);

await user.click( dayButton( new Date( 2026, 0, 15 ) ) );
await flushTimeouts();

expect( onChange ).toHaveBeenCalledTimes( 1 );
// The time is preserved from the previous value.
expect( onChange ).toHaveBeenCalledWith( {
date: '2026-01-15T10:00:00.000Z',
} );
expect( speak ).not.toHaveBeenCalled();
} );

it( 'should call onChange once and show the required error when the date is cleared, keeping focus on the day button', async () => {
const onChange = jest.fn();
const user = userEvent.setup();

render( <ControlledForm onChange={ onChange } /> );

// Clicking the selected day deselects it.
await user.click( dayButton( new Date( 2026, 0, 10 ) ) );
await flushTimeouts();

expect( onChange ).toHaveBeenCalledTimes( 1 );
expect( onChange ).toHaveBeenCalledWith( { date: undefined } );
expect(
await screen.findByText( 'Constraints not satisfied' )
).toBeVisible();
// Focus does not move, so the error is announced instead.
expect( speak ).toHaveBeenCalledWith( 'Constraints not satisfied' );
expect( dayButton( new Date( 2026, 0, 10 ) ) ).toHaveFocus();
} );

it( 'should clear the revealed error when a valid date is selected in the calendar', async () => {
const user = userEvent.setup();

render( <ControlledForm /> );

// Clicking the selected day deselects it, making the field invalid.
await user.click( dayButton( new Date( 2026, 0, 10 ) ) );
await flushTimeouts();

expect(
await screen.findByText( 'Constraints not satisfied' )
).toBeVisible();

await user.click( dayButton( new Date( 2026, 0, 15 ) ) );
await flushTimeouts();

expect(
screen.queryByText( 'Constraints not satisfied' )
).not.toBeInTheDocument();
} );
} );
} );
Loading