Skip to content

Commit

Permalink
code review feedback
Browse files Browse the repository at this point in the history
- `openOnClick` will keep the input focused
- arrow key down will move focus to the calendar
  • Loading branch information
mark-tate committed Jan 30, 2025
1 parent 5719eaa commit 5d0a456
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 96 deletions.
3 changes: 2 additions & 1 deletion .changeset/serious-kings-decide.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

enabled uncontrolled/un-controlled open behaviour for `DatePicker`

- added `openOnClick` and `openOnKeyDown` props to `DatePicker`.
- added `openOnClick` props to `DatePicker`.
- when the triggering element (`DateInput`) is focused, arrow key down, will now open the DatePicker by default
- revise the controlled behaviour of the `open` prop on `DatePickerOverlay`.
- add examples for controlled and uncontrolled behaviour.
13 changes: 1 addition & 12 deletions packages/lab/src/date-picker/DatePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ export interface DatePickerBaseProps {
open?: boolean;
/** When `open` is uncontrolled, set this to `true` to open on click */
openOnClick?: boolean;
/** When `open` is uncontrolled, set this to `true` to open on arrow key down */
openOnKeyDown?: boolean;
/**
* Handler for when open state changes
* @param newOpen - true when opened
Expand Down Expand Up @@ -128,22 +126,13 @@ export const DatePickerMain = forwardRef<HTMLDivElement, DatePickerProps<any>>(
export const DatePicker = forwardRef(function DatePicker<
TDate extends DateFrameworkType,
>(props: DatePickerProps<TDate>, ref: React.Ref<HTMLDivElement>) {
const {
defaultOpen,
open,
openOnClick,
openOnKeyDown,
onOpen,
readOnly,
...rest
} = props;
const { defaultOpen, open, openOnClick, onOpen, readOnly, ...rest } = props;

return (
<DatePickerOverlayProvider
defaultOpen={defaultOpen}
open={open}
openOnClick={openOnClick}
openOnKeyDown={openOnKeyDown}
onOpen={onOpen}
readOnly={readOnly}
>
Expand Down
9 changes: 0 additions & 9 deletions packages/lab/src/date-picker/DatePickerOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,6 @@ export const DatePickerOverlay = forwardRef<
role="dialog"
aria-modal="true"
ref={floatingRef}
focusManagerProps={
floatingUIResult?.context
? {
returnFocus: false,
context: floatingUIResult.context,
initialFocus: 4,
}
: undefined
}
{...(getFloatingProps
? getFloatingProps({
...a11yProps,
Expand Down
53 changes: 25 additions & 28 deletions packages/lab/src/date-picker/DatePickerOverlayProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
useContext,
useMemo,
useRef,
useState,
} from "react";
import { useKeyboard } from "./useKeyboard";

Expand Down Expand Up @@ -46,8 +47,10 @@ interface DatePickerOverlayHelpers {
/**
* Sets the open state of the overlay.
* @param newOpen - The new value for the open state.
* @param event - Event which triggered the change of state.
* @param reason - Reason for the state change
*/
setOpen: (newOpen: boolean) => void;
setOpen: (newOpen: boolean, event?: Event, reason?: OpenChangeReason) => void;
/**~
* Register a callback for when onDismiss is called
* @param onDismissCallback
Expand Down Expand Up @@ -88,10 +91,6 @@ interface DatePickerOverlayProviderProps {
* When `open` is uncontrolled, set this to `true` to open on click
*/
openOnClick?: boolean;
/**
* When `open` is uncontrolled, set this to `true` to open on arrow key down
*/
openOnKeyDown?: boolean;
/**
* Handler for when open state changes
* @param newOpen - true when opened
Expand Down Expand Up @@ -120,7 +119,6 @@ export const DatePickerOverlayProvider: React.FC<
> = ({
open: openProp,
openOnClick,
openOnKeyDown = true,
defaultOpen,
onOpen,
children,
Expand All @@ -133,18 +131,19 @@ export const DatePickerOverlayProvider: React.FC<
name: "DatePicker",
state: "openDatePickerOverlay",
});
const triggeringElement = useRef<HTMLElement | null>(null);
const [disableFocus, setDisableFocus] = useState<boolean>(true);
const triggeringElementRef = useRef<HTMLElement | null>(null);
const onDismissCallback = useRef<() => void>();

const setOpen = useCallback(
(newOpen: boolean, _event?: Event, reason?: OpenChangeReason) => {
if (newOpen) {
if (readOnly) {
return;
}
triggeringElement.current = document.activeElement as HTMLElement;
triggeringElementRef.current = document.activeElement as HTMLElement;
setDisableFocus(reason === "click"); // prevent the overlay taking focus on click
} else if (!isOpenControlled) {
const trigger = triggeringElement.current as HTMLElement;
const trigger = triggeringElementRef.current as HTMLElement;
if (trigger) {
trigger.focus();
}
Expand All @@ -153,9 +152,8 @@ export const DatePickerOverlayProvider: React.FC<
trigger.setSelectionRange(0, trigger.value.length);
}, 1);
}
triggeringElement.current = null;
triggeringElementRef.current = null;
}

setOpenState(newOpen);
onOpen?.(newOpen);

Expand All @@ -177,45 +175,44 @@ export const DatePickerOverlayProvider: React.FC<
});

const {
getFloatingProps: _getFloatingPropsCallback,
getReferenceProps: _getReferenceProps,
getFloatingProps: getFloatingPropsCallback,
getReferenceProps: getReferencePropsCallback,
} = useInteractions(
interactions
? interactions(floatingUIResult.context)
: [
useDismiss(floatingUIResult.context),
useKeyboard(floatingUIResult.context, {
enabled: !!openOnKeyDown && !readOnly,
enabled: !readOnly,
}),
useClick(floatingUIResult.context, {
enabled: !!openOnClick && !readOnly,
toggle: false,
keyboardHandlers: false,
}),
],
);

const getFloatingPropsCallback = useMemo(
() => _getFloatingPropsCallback,
[_getFloatingPropsCallback],
);
const getReferenceProps = useMemo(
() => _getReferenceProps,
[_getReferenceProps],
);

const getFloatingProps = useCallback(
(userProps: React.HTMLProps<HTMLElement> | undefined) => {
const { x, y, strategy, elements } = floatingUIResult;
const floatingProps = getFloatingPropsCallback(userProps);
return {
top: y ?? 0,
left: x ?? 0,
position: strategy,
width: elements.floating?.offsetWidth,
height: elements.floating?.offsetHeight,
...getFloatingPropsCallback(userProps),
...floatingProps,
focusManagerProps: {
disabled: disableFocus,
returnFocus: false,
context: floatingUIResult.context,
initialFocus: 4,
},
};
},
[getFloatingPropsCallback, floatingUIResult],
[getFloatingPropsCallback, floatingUIResult, disableFocus],
);
const setOnDismiss = useCallback((dismissCallback: () => void) => {
onDismissCallback.current = dismissCallback;
Expand All @@ -232,11 +229,11 @@ export const DatePickerOverlayProvider: React.FC<
const helpers: DatePickerOverlayHelpers = useMemo(
() => ({
getFloatingProps,
getReferenceProps,
getReferenceProps: getReferencePropsCallback,
setOpen,
setOnDismiss,
}),
[getFloatingProps, getReferenceProps, setOpen],
[getFloatingProps, getReferencePropsCallback, setOpen],
);
const contextValue = useMemo(() => ({ state, helpers }), [state, helpers]);

Expand Down
12 changes: 9 additions & 3 deletions packages/lab/src/date-picker/DatePickerRangeInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from "@salt-ds/date-adapters";
import { clsx } from "clsx";
import {
type MouseEventHandler,
type SyntheticEvent,
forwardRef,
useCallback,
Expand Down Expand Up @@ -150,9 +151,14 @@ export const DatePickerRangeInput = forwardRef(function DatePickerRangeInput<
state: "dateValue",
});

const handleCalendarButton = useCallback(() => {
setOpen(!open);
}, [open, setOpen]);
const handleCalendarButton: MouseEventHandler<HTMLButtonElement> =
useCallback(
(event) => {
setOpen(!open);
event.stopPropagation();
},
[open, setOpen],
);

const handleDateChange = useCallback(
(
Expand Down
12 changes: 9 additions & 3 deletions packages/lab/src/date-picker/DatePickerSingleInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import { CalendarIcon } from "@salt-ds/icons";
import { clsx } from "clsx";
import {
type MouseEventHandler,
type SyntheticEvent,
forwardRef,
useCallback,
Expand Down Expand Up @@ -120,9 +121,14 @@ export const DatePickerSingleInput = forwardRef<
state: "value",
});

const handleCalendarButton = useCallback(() => {
setOpen(!open);
}, [open, setOpen]);
const handleCalendarButton: MouseEventHandler<HTMLButtonElement> =
useCallback(
(event) => {
setOpen(!open);
event.stopPropagation();
},
[open, setOpen],
);

const handleDateChange = useCallback(
(
Expand Down
6 changes: 5 additions & 1 deletion packages/lab/src/date-picker/DatePickerSinglePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { Calendar, type SingleDateSelection } from "../calendar";
import { useLocalization } from "../localization-provider";
import { useDatePickerContext } from "./DatePickerContext";
import { useDatePickerOverlay } from "./DatePickerOverlayProvider";
import datePickerPanelCss from "./DatePickerPanel.css";

/**
Expand Down Expand Up @@ -138,6 +139,9 @@ export const DatePickerSinglePanel = forwardRef(function DatePickerSinglePanel<
helpers: { select },
} = useDatePickerContext<TDate>({ selectionVariant: "single" });

const {
state: { initialFocusRef },

Check failure on line 143 in packages/lab/src/date-picker/DatePickerSinglePanel.tsx

View workflow job for this annotation

GitHub Actions / type-checks

Property 'initialFocusRef' does not exist on type 'DatePickerOverlayState'.
} = useDatePickerOverlay();
const [hoveredDate, setHoveredDate] = useState<TDate | null>(null);

const [uncontrolledDefaultVisibleMonth] = useState(() => {
Expand Down Expand Up @@ -217,7 +221,7 @@ export const DatePickerSinglePanel = forwardRef(function DatePickerSinglePanel<
<Calendar selectionVariant="single" {...baseCalendarProps}>
<CalendarNavigation {...CalendarNavigationProps} />
<CalendarWeekHeader {...CalendarWeekHeaderProps} />
<CalendarGrid {...CalendarDataGridProps} />
<CalendarGrid ref={initialFocusRef} {...CalendarDataGridProps} />
</Calendar>
</FormFieldContext.Provider>
</FlexLayout>
Expand Down
53 changes: 37 additions & 16 deletions packages/lab/stories/date-picker/date-picker.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2695,11 +2695,10 @@ WithExperimentalTime.parameters = {
},
};

export const UncontrolledOpen: StoryFn<
export const UncontrolledSingleOpen: StoryFn<
DatePickerSingleProps<DateFrameworkType>
> = ({ selectionVariant, defaultSelectedDate, ...args }) => {
const [openOnClick, setOpenOnClick] = useState(false);
const [openOnKeyDown, setOpenOnKeyDown] = useState(false);
return (
<StackLayout style={{ width: "400px" }}>
<FlexLayout>
Expand All @@ -2712,28 +2711,52 @@ export const UncontrolledOpen: StoryFn<
>
Open On Click
</ToggleButton>
</FlexLayout>
<DatePicker
selectionVariant={"single"}
openOnClick={openOnClick}
{...args}
defaultSelectedDate={defaultSelectedDate}
>
<DatePickerTrigger>
<DatePickerSingleInput />
</DatePickerTrigger>
<DatePickerOverlay>
<DatePickerSinglePanel />
</DatePickerOverlay>
</DatePicker>
</StackLayout>
);
};

export const UncontrolledRangeOpen: StoryFn<
DatePickerRangeProps<DateFrameworkType>
> = ({ selectionVariant, defaultSelectedDate, ...args }) => {
const [openOnClick, setOpenOnClick] = useState(false);
return (
<StackLayout style={{ width: "400px" }}>
<FlexLayout>
<ToggleButton
aria-label={"open on key down"}
value={openOnKeyDown ? "false" : "true"}
aria-label={"open on click"}
value={openOnClick ? "false" : "true"}
onChange={(event) =>
setOpenOnKeyDown(event.currentTarget.value === "true")
setOpenOnClick(event.currentTarget.value === "true")
}
>
Open On Key Down
Open On Click
</ToggleButton>
</FlexLayout>
<DatePicker
selectionVariant={"single"}
selectionVariant={"range"}
openOnClick={openOnClick}
openOnKeyDown={openOnKeyDown}
{...args}
defaultSelectedDate={defaultSelectedDate}
>
<DatePickerTrigger>
<DatePickerSingleInput />
<DatePickerRangeInput />
</DatePickerTrigger>
<DatePickerOverlay>
<DatePickerSinglePanel />
<DatePickerRangePanel />
</DatePickerOverlay>
</DatePicker>
</StackLayout>
Expand Down Expand Up @@ -2800,16 +2823,14 @@ export const ControlledOpen: StoryFn<
return (
<StackLayout style={{ width: "400px" }}>
<FlexLayout>
<ToggleButton
<Button
aria-label={"open picker"}
value={open ? "false" : "true"}
selected={open}
onChange={(event) => {
setOpen(event.currentTarget.value === "true");
onClick={() => {
setOpen(true);
}}
>
Open
</ToggleButton>
</Button>
</FlexLayout>
<DatePicker
selectionVariant={"single"}
Expand Down
2 changes: 1 addition & 1 deletion site/docs/components/date-picker/examples.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ A `DatePicker` component with a border provides a visually distinct area for sel

## Uncontrolled open

By default, the overlay's open state is uncontrolled and opens only when the calendar button is used. However, it can also be configured to open using the `openOnClick` and `openOnKeyDown`props.
By default, the overlay's open state is uncontrolled and opens only when the calendar button or down arrow is used. However, it can also be configured to open using the `openOnClick` prop.

</LivePreview>

Expand Down
Loading

0 comments on commit 5d0a456

Please sign in to comment.