Components: Reuse existing `useControlledValue` hook in Calendar components #70655

chihsuan · 2025-07-09T08:18:21Z

What?

This PR consolidates the useControlledValue hook implementation by enhancing the existing useControlledValue hook and removing the duplicate Calendar-specific version.

Why?

cc @ciampo

Follow-up to #70578 addressing this review comment to reuse the existing useControlledValue hook in the package instead of creating a duplicate implementation.

This change:

Eliminates code duplication between the shared and Calendar-specific hooks
Provides a single, consistent implementation across all components
Reduces maintenance overhead

How?

The key differences between these two useControlledValue hooks are:

1. Type signature:

Value:

Utils version: value?: T
Calendar version: value?: T | null | undefined

onChange:

Utils version: onChange?: (value: T) => void
Calendar version: onChange?: (newValue: T, ...args: any[]) => void

2. Value handling:

Utils version: const value = hasValue ? valueProp : state;
Calendar version: const value = (hasValue ? valueProp : state) ?? undefined;

3. Implementation approach:

Utils version: Uses conditional assignment for setValue
Calendar version: Uses useCallback for uncontrolled mode with uncontrolledSetValue

Overall, the calendar version is more specialized for date handling (null values) and supports additional arguments in onChange callbacks.

To reuse the existing useControlledValue hook in the Calendar components, I made the following changes to the shared version:

Added support for additional arguments in onChange callbacks (...args).
Used useCallback for uncontrolled mode. This is not strictly necessary for Calendar, but it's a good practice to use it IMO. If there is a concern about introducing this change, we can revert it.

And then, I updated the Calendar components:

Removed the duplicate Calendar-specific implementation
Updated Calendar components to import from the shared hook location
Converted null values to undefined for selected value in Calendar components to ensure correct behavior.

Testing Instructions

Note: Existing Calendar component tests have covered controlled/uncontrolled modes, onChange callbacks with additional arguments, and state transitions.

Run npm run storybook:dev
Go to http://localhost:50240/?path=/story/components-selection-input-time-date-datecalendar--default
Verify Calendar components (DateCalendar, DateRangeCalendar) continue to work as expected
Test both controlled and uncontrolled modes in Calendar components by setting defaultSelected and selected props.

Testing Instructions for Keyboard

github-actions · 2025-07-09T08:18:31Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: chihsuan <chihsuan@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

chihsuan · 2025-07-09T08:21:48Z

packages/components/src/calendar/stories/date-calendar.story.tsx

+		defaultSelected: new Date( 1751472000000 ),
+		selected: new Date( 1751558400000 ),
+	},
+};


Fixed TS errors. I'm not sure why these TS errors weren't caught in the previous PR.

gutenberg/packages/components/src/calendar/types.ts

Lines 310 to 318 in 9504e40

interface SingleProps {

/** The selected date. */

selected?: Date | undefined | null;

/** Event handler when a day is selected. */

onSelect?: OnSelectHandler< Date | undefined >;

/** The default selected date (for uncontrolled usage). */

defaultSelected?: Date;

}

Which errors? AFAIK it's fine for DateCalendar not to have those props defined. Besides, it wouldn't make sense to define both defaultSelected and selected at the same time — one is for uncontrolled usage, the other one for controlled usage.

We should be able to undo these changes without causing TS errors

You're right! Previously, I encountered TS errors that stopped me from committing the changes. But now, I don't see any errors. I'll undo the changes.

Reverted e66276b

chihsuan · 2025-07-09T08:24:00Z

packages/components/src/calendar/date-calendar/index.tsx

@@ -48,7 +51,7 @@ export const DateCalendar = ( {
 			{ ...props }
 			mode="single"
 			numberOfMonths={ clampNumberOfMonths( numberOfMonths ) }
-			selected={ selected }
+			selected={ selected ?? undefined }


Previously, the calendar's useControlledValue hook internally converted the empty value to undefined. I don't think this should be added to the utils useControlledValue, so I updated the calendar's implementation to handle this case.

gutenberg/packages/components/src/calendar/utils/use-controlled-value.ts

Line 39 in 2dd93a3

const value = ( hasValue ? valueProp : state ) ?? undefined;

- Updated the `onChange` prop to accept additional arguments for better flexibility. - Introduced a new `uncontrolledSetValue` function using `useCallback` to streamline state updates and onChange calls in uncontrolled mode. - Improved code clarity by separating controlled and uncontrolled mode logic. This change enhances the usability of the hook in various scenarios.

…hook - Updated imports of `useControlledValue` in `DateCalendar` and `DateRangeCalendar` components to point to the new location in `../../utils/hooks`. - Enhanced type handling for the `selected` state to accommodate `null` values, ensuring better compatibility with the updated hook. - Adjusted the `selected` prop to default to `undefined` when `null`, improving the handling of unselected states. - Modified the `Default` story for `DateCalendar` to include example arguments for `defaultSelected` and `selected`, enhancing story clarity.

ciampo · 2025-07-10T16:53:49Z

Thank you, @chihsuan ! I'll take a look in the upcoming days :)

ciampo

Thank you for working on this! I suggested a few tweaks, but the overall approach is good :)

ciampo · 2025-07-14T10:10:41Z

packages/components/src/calendar/stories/date-calendar.story.tsx

+		defaultSelected: new Date( 1751472000000 ),
+		selected: new Date( 1751558400000 ),
+	},
+};


Which errors? AFAIK it's fine for DateCalendar not to have those props defined. Besides, it wouldn't make sense to define both defaultSelected and selected at the same time — one is for uncontrolled usage, the other one for controlled usage.

We should be able to undo these changes without causing TS errors

ciampo · 2025-07-14T10:12:50Z

packages/components/src/calendar/date-calendar/index.tsx

+	const [ selected, setSelected ] = useControlledValue<
+		Date | undefined | null
+	>( {
 		defaultValue: defaultSelected,
 		value: selectedProp,
+		// @ts-ignore: The useControlledValue expects onChange value parameter type is the same as the value parameter, but in our case, onSelect always receives DateRange | undefined.


Similarly to how we now handle the fallback to ?? undefined when passing the selected value to DayPicker, we can write an internal onChange callback to do the same conversion and avoid the @ts-ignore

diff --git i/packages/components/src/calendar/date-calendar/index.tsx w/packages/components/src/calendar/date-calendar/index.tsx index a77da53060..f70807d4d3 100644 --- i/packages/components/src/calendar/date-calendar/index.tsx +++ w/packages/components/src/calendar/date-calendar/index.tsx @@ -3,6 +3,10 @@ */ import { DayPicker } from 'react-day-picker'; import { enUS } from 'react-day-picker/locale'; +/** + * WordPress dependencies + */ +import { useCallback } from '@wordpress/element'; /** * Internal dependencies */ @@ -10,7 +14,7 @@ import { COMMON_PROPS } from '../utils/constants'; import { clampNumberOfMonths } from '../utils/misc'; import { useControlledValue } from '../../utils/hooks'; import { useLocalizationProps } from '../utils/use-localization-props'; -import type { DateCalendarProps } from '../types'; +import type { DateCalendarProps, OnSelectHandler } from '../types'; /** * `DateCalendar` is a React component that provides a customizable calendar @@ -35,14 +39,21 @@ export const DateCalendar = ( { mode: 'single', } ); - const [ selected, setSelected ] = useControlledValue< - Date | undefined | null - >( { - defaultValue: defaultSelected, - value: selectedProp, - // @ts-ignore: The useControlledValue expects onChange value parameter type is the same as the value parameter, but in our case, onSelect always receives DateRange | undefined. - onChange: onSelect, - } ); + const onChange: OnSelectHandler< typeof selectedProp > = useCallback( + ( selected, triggerDate, modifiers, e ) => { + // Convert internal `null` to `undefined` for the public event handler. + onSelect?.( selected ?? undefined, triggerDate, modifiers, e ); + }, + [ onSelect ] + ); + + const [ selected, setSelected ] = useControlledValue< typeof selectedProp >( + { + defaultValue: defaultSelected, + value: selectedProp, + onChange, + } + ); return ( <DayPicker diff --git i/packages/components/src/calendar/date-range-calendar/index.tsx w/packages/components/src/calendar/date-range-calendar/index.tsx index 0d4871103b..2b40e3c0a4 100644 --- i/packages/components/src/calendar/date-range-calendar/index.tsx +++ w/packages/components/src/calendar/date-range-calendar/index.tsx @@ -7,7 +7,7 @@ import { enUS } from 'react-day-picker/locale'; /** * WordPress dependencies */ -import { useMemo, useState } from '@wordpress/element'; +import { useMemo, useState, useCallback } from '@wordpress/element'; /** * Internal dependencies */ @@ -15,7 +15,11 @@ import { COMMON_PROPS, MODIFIER_CLASSNAMES } from '../utils/constants'; import { clampNumberOfMonths } from '../utils/misc'; import { useControlledValue } from '../../utils/hooks'; import { useLocalizationProps } from '../utils/use-localization-props'; -import type { DateRangeCalendarProps, DateRange } from '../types'; +import type { + DateRangeCalendarProps, + DateRange, + OnSelectHandler, +} from '../types'; export function usePreviewRange( { selected, @@ -151,13 +155,20 @@ export const DateRangeCalendar = ( { mode: 'range', } ); + const onChange: OnSelectHandler< typeof selectedProp > = useCallback( + ( selected, triggerDate, modifiers, e ) => { + // Convert internal `null` to `undefined` for the public event handler. + onSelect?.( selected ?? undefined, triggerDate, modifiers, e ); + }, + [ onSelect ] + ); + const [ selected, setSelected ] = useControlledValue< DateRange | undefined | null >( { defaultValue: defaultSelected, value: selectedProp, - // @ts-ignore: The useControlledValue expects onChange value parameter type is the same as the value parameter, but in our case, onSelect always receives DateRange | undefined. - onChange: onSelect, + onChange, } ); const [ hoveredDate, setHoveredDate ] = useState< Date | undefined >( diff --git i/packages/components/src/calendar/types.ts w/packages/components/src/calendar/types.ts index dc4bb72d5b..931556aaf5 100644 --- i/packages/components/src/calendar/types.ts +++ w/packages/components/src/calendar/types.ts @@ -153,7 +153,7 @@ type DayOfWeek = { * @param {Modifiers} modifiers - The modifiers associated with the event. * @param {React.MouseEvent | React.KeyboardEvent} e - The event object. */ -type OnSelectHandler< T > = ( +export type OnSelectHandler< T > = ( selected: T, triggerDate: Date, modifiers: Modifiers, diff --git i/packages/components/src/utils/hooks/use-controlled-value.ts w/packages/components/src/utils/hooks/use-controlled-value.ts index b43f670332..725283736f 100644 --- i/packages/components/src/utils/hooks/use-controlled-value.ts +++ w/packages/components/src/utils/hooks/use-controlled-value.ts @@ -48,5 +48,5 @@ export function useControlledValue< T >( { setValue = setState; } - return [ value, setValue as typeof setState ] as const; + return [ value, setValue ] as const; }

Thank you for the detailed feedback and the comprehensive diff!

I initially went with @ts-ignore to keep the changes minimal, but after considering your approach, I agree that the explicit onChange callback is much better for several.

Updated as suggested in 8db0b82

…backs

chihsuan · 2025-07-16T05:31:45Z

packages/components/src/toggle-group-control/toggle-group-control/as-button-group.tsx

@@ -48,7 +48,9 @@ function UnforwardedToggleGroupControlAsButtonGroup(
 	const { value, defaultValue } =
 		useComputeControlledOrUncontrolledValue( valueProp );

-	const [ selectedValue, setSelectedValue ] = useControlledValue( {
+	const [ selectedValue, setSelectedValue ] = useControlledValue<
+		typeof value


We removed as typeof setState from the useControlledValue hook, which caused a TS error. This was resolved by explicitly specifying the generic type for useControlledValue.

https://github.com/WordPress/gutenberg/pull/70655/files#diff-4df4aa449ece25eb703b298c9bfc3cdaae55552ccd2040f629efc4caa1bf9f8fL43

chihsuan · 2025-07-16T05:33:13Z

@ciampo Thank you for the review! I've addressed all the review comments. All changes are ready for another review. 🙏

ciampo · 2025-07-17T11:27:43Z

packages/components/src/calendar/date-range-calendar/index.tsx

+		},
+		[ onSelect ]
+	);
+
 	const [ selected, setSelected ] = useControlledValue<
 		DateRange | undefined | null


Nit: could this be also changed to typeof selectedProp ?

As a further improvement, it would be nice to make the useControlledValue hook infer the T type from the received arguments, but that's definitely something we don't have to work on today.

Nit: could this be also changed to typeof selectedProp ?

Abosolutely, changed it to typeof selectedProp in 8c3977f. 🙌

ciampo

LGTM 🚀 We can merge once this last comment is addressed

chihsuan · 2025-07-18T02:25:55Z

We can merge once #70655 (comment) is addressed

Thank you, @ciampo! I've made the changes. Since I don't have merge permissions, could you or someone else please handle the merge? Much appreciated! 🙏

@ts-ignore

…onents (WordPress#70655) * Refactor `useControlledValue` hook to enhance onChange handling - Updated the `onChange` prop to accept additional arguments for better flexibility. - Introduced a new `uncontrolledSetValue` function using `useCallback` to streamline state updates and onChange calls in uncontrolled mode. - Improved code clarity by separating controlled and uncontrolled mode logic. This change enhances the usability of the hook in various scenarios. * Refactor calendar components to utilize updated `useControlledValue` hook - Updated imports of `useControlledValue` in `DateCalendar` and `DateRangeCalendar` components to point to the new location in `../../utils/hooks`. - Enhanced type handling for the `selected` state to accommodate `null` values, ensuring better compatibility with the updated hook. - Adjusted the `selected` prop to default to `undefined` when `null`, improving the handling of unselected states. - Modified the `Default` story for `DateCalendar` to include example arguments for `defaultSelected` and `selected`, enhancing story clarity. * Improve docs * Add changelog * Remove the args from DateCalendar story * Address review feedback: Replace @ts-ignore with proper onChange callbacks * Update useControlledValue type to match selectedProp

chihsuan requested a review from ajitbohra as a code owner July 9, 2025 08:18

chihsuan commented Jul 9, 2025

View reviewed changes

chihsuan force-pushed the enhance/calendar-reuse-use-controlled-value branch from 66c8e75 to 24b85da Compare July 9, 2025 08:30

chihsuan added 4 commits July 10, 2025 10:31

Improve docs

ce8284a

Add changelog

b9cc963

chihsuan force-pushed the enhance/calendar-reuse-use-controlled-value branch from 24b85da to b9cc963 Compare July 10, 2025 02:31

t-hamano added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Jul 10, 2025

ciampo requested review from mirka and ciampo July 10, 2025 16:53

ciampo assigned ciampo and chihsuan and unassigned ciampo Jul 10, 2025

ciampo reviewed Jul 14, 2025

View reviewed changes

chihsuan added 3 commits July 16, 2025 13:06

Fix conflicts

83a72cf

Remove the args from DateCalendar story

e66276b

Address review feedback: Replace @ts-ignore with proper onChange call…

8db0b82

…backs

chihsuan requested a review from ciampo July 16, 2025 05:28

chihsuan commented Jul 16, 2025

View reviewed changes

ciampo reviewed Jul 17, 2025

View reviewed changes

ciampo approved these changes Jul 17, 2025

View reviewed changes

Update useControlledValue type to match selectedProp

8c3977f

ciampo enabled auto-merge (squash) July 18, 2025 09:14

ciampo merged commit a3df6d4 into WordPress:trunk Jul 18, 2025
59 checks passed

github-actions bot added this to the Gutenberg 21.3 milestone Jul 18, 2025


	interface SingleProps {
	/** The selected date. */
	selected?: Date \| undefined \| null;
	/** Event handler when a day is selected. */
	onSelect?: OnSelectHandler< Date \| undefined >;
	/** The default selected date (for uncontrolled usage). */
	defaultSelected?: Date;
	}

Components: Reuse existing useControlledValue hook in Calendar components #70655

Components: Reuse existing useControlledValue hook in Calendar components #70655

Conversation

chihsuan commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Uh oh!

github-actions bot commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciampo commented Jul 10, 2025

Uh oh!

ciampo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chihsuan commented Jul 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciampo left a comment

Choose a reason for hiding this comment

Uh oh!

chihsuan commented Jul 18, 2025

Uh oh!

Uh oh!

Uh oh!

Components: Reuse existing `useControlledValue` hook in Calendar components #70655

Components: Reuse existing `useControlledValue` hook in Calendar components #70655

chihsuan commented Jul 9, 2025 •

edited

Loading

github-actions bot commented Jul 9, 2025 •

edited

Loading