Skip to content

Commit

Permalink
[pickers] Fix views management (@LukasTy) (#11572)
Browse files Browse the repository at this point in the history
Co-authored-by: Lukas <llukas.tyla@gmail.com>
  • Loading branch information
github-actions[bot] and LukasTy committed Jan 4, 2024
1 parent 2af7904 commit 0678e0a
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 87 deletions.
4 changes: 2 additions & 2 deletions docs/pages/x/api/date-pickers/date-calendar.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
"onChange": {
"type": { "name": "func" },
"signature": {
"type": "function(value: TDate | null, selectionState: PickerSelectionState | undefined) => void",
"describedArgs": ["value", "selectionState"]
"type": "function(value: TValue, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"describedArgs": ["value", "selectionState", "selectedView"]
}
},
"onFocusedViewChange": {
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/x/api/date-pickers/digital-clock.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"onChange": {
"type": { "name": "func" },
"signature": {
"type": "function(value: TDate | null, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"type": "function(value: TValue, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"describedArgs": ["value", "selectionState", "selectedView"]
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"onChange": {
"type": { "name": "func" },
"signature": {
"type": "function(value: TDate | null, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"type": "function(value: TValue, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"describedArgs": ["value", "selectionState", "selectedView"]
}
},
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/x/api/date-pickers/time-clock.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"onChange": {
"type": { "name": "func" },
"signature": {
"type": "function(value: TDate | null, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"type": "function(value: TValue, selectionState: PickerSelectionState | undefined, selectedView: TView | undefined) => void",
"describedArgs": ["value", "selectionState", "selectedView"]
}
},
Expand Down
3 changes: 2 additions & 1 deletion docs/translations/api-docs/date-pickers/date-calendar.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@
"deprecated": "",
"typeDescriptions": {
"value": "The new value.",
"selectionState": "Indicates if the date selection is complete."
"selectionState": "Indicates if the date selection is complete.",
"selectedView": "Indicates the view in which the selection has been made."
}
},
"onFocusedViewChange": {
Expand Down
14 changes: 10 additions & 4 deletions packages/x-date-pickers/src/DateCalendar/DateCalendar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,14 @@ export const DateCalendar = React.forwardRef(function DateCalendar<TDate>(
const handleSelectedDayChange = useEventCallback((day: TDate | null) => {
if (day) {
// If there is a date already selected, then we want to keep its time
return handleValueChange(mergeDateAndTime(utils, day, value ?? referenceDate), 'finish');
return handleValueChange(
mergeDateAndTime(utils, day, value ?? referenceDate),
'finish',
view,
);
}

return handleValueChange(day, 'finish');
return handleValueChange(day, 'finish', view);
});

React.useEffect(() => {
Expand Down Expand Up @@ -505,9 +509,11 @@ DateCalendar.propTypes = {
monthsPerRow: PropTypes.oneOf([3, 4]),
/**
* Callback fired when the value changes.
* @template TDate
* @param {TDate | null} value The new value.
* @template TValue The value type. Will be either the same type as `value` or `null`. Can be in `[start, end]` format in case of range value.
* @template TView The view type. Will be one of date or time views.
* @param {TValue} value The new value.
* @param {PickerSelectionState | undefined} selectionState Indicates if the date selection is complete.
* @param {TView | undefined} selectedView Indicates the view in which the selection has been made.
*/
onChange: PropTypes.func,
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
MonthValidationProps,
DayValidationProps,
} from '../internals/models/validation';
import { PickerSelectionState } from '../internals/hooks/usePicker/usePickerValue.types';
import { ExportedUseViewsOptions } from '../internals/hooks/useViews';
import { DateView, TimezoneProps } from '../models';
import { DefaultizedProps } from '../internals/models/helpers';
Expand Down Expand Up @@ -113,13 +112,6 @@ export interface DateCalendarProps<TDate>
* @default The closest valid date using the validation props, except callbacks such as `shouldDisableDate`.
*/
referenceDate?: TDate;
/**
* Callback fired when the value changes.
* @template TDate
* @param {TDate | null} value The new value.
* @param {PickerSelectionState | undefined} selectionState Indicates if the date selection is complete.
*/
onChange?: (value: TDate | null, selectionState?: PickerSelectionState) => void;
className?: string;
classes?: Partial<DateCalendarClasses>;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('<DesktopDateTimePicker />', () => {
/>,
);

openPicker({ type: 'date', variant: 'desktop' });
openPicker({ type: 'date-time', variant: 'desktop' });

// Select year
userEvent.mousePress(screen.getByRole('radio', { name: '2025' }));
Expand All @@ -64,6 +64,48 @@ describe('<DesktopDateTimePicker />', () => {
});
});

it('should allow selecting same view multiple times', () => {
const onChange = spy();
const onAccept = spy();
const onClose = spy();

render(
<DesktopDateTimePicker
onChange={onChange}
onAccept={onAccept}
onClose={onClose}
defaultValue={adapterToUse.date('2018-01-01T11:55:00')}
/>,
);

openPicker({ type: 'date-time', variant: 'desktop' });

// Change the date multiple times to check that picker doesn't close after cycling through all views internally
userEvent.mousePress(screen.getByRole('gridcell', { name: '2' }));
userEvent.mousePress(screen.getByRole('gridcell', { name: '3' }));
userEvent.mousePress(screen.getByRole('gridcell', { name: '4' }));
userEvent.mousePress(screen.getByRole('gridcell', { name: '5' }));
expect(onChange.callCount).to.equal(4);
expect(onAccept.callCount).to.equal(0);
expect(onClose.callCount).to.equal(0);

// Change the hours
userEvent.mousePress(screen.getByRole('option', { name: '10 hours' }));
userEvent.mousePress(screen.getByRole('option', { name: '9 hours' }));
expect(onChange.callCount).to.equal(6);
expect(onAccept.callCount).to.equal(0);
expect(onClose.callCount).to.equal(0);

// Change the minutes
userEvent.mousePress(screen.getByRole('option', { name: '50 minutes' }));
expect(onChange.callCount).to.equal(7);
// Change the meridiem
userEvent.mousePress(screen.getByRole('option', { name: 'PM' }));
expect(onChange.callCount).to.equal(8);
expect(onAccept.callCount).to.equal(1);
expect(onClose.callCount).to.equal(1);
});

describe('prop: timeSteps', () => {
it('should use "DigitalClock" view renderer, when "timeSteps.minutes" = 60', () => {
const onChange = spy();
Expand Down
5 changes: 3 additions & 2 deletions packages/x-date-pickers/src/DigitalClock/DigitalClock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,9 @@ DigitalClock.propTypes = {
minutesStep: PropTypes.number,
/**
* Callback fired when the value changes.
* @template TDate, TView
* @param {TDate | null} value The new value.
* @template TValue The value type. Will be either the same type as `value` or `null`. Can be in `[start, end]` format in case of range value.
* @template TView The view type. Will be one of date or time views.
* @param {TValue} value The new value.
* @param {PickerSelectionState | undefined} selectionState Indicates if the date selection is complete.
* @param {TView | undefined} selectedView Indicates the view in which the selection has been made.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,10 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
return inViews.includes('meridiem') ? inViews : [...inViews, 'meridiem'];
}, [ampm, inViews]);

const { view, setValueAndGoToView, focusedView } = useViews<TDate | null, TimeViewWithMeridiem>({
const { view, setValueAndGoToNextView, focusedView } = useViews<
TDate | null,
TimeViewWithMeridiem
>({
view: inView,
views,
openTo,
Expand All @@ -162,7 +165,7 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
});

const handleMeridiemValueChange = useEventCallback((newValue: TDate | null) => {
setValueAndGoToView(newValue, null, 'meridiem');
setValueAndGoToNextView(newValue, 'finish', 'meridiem');
});

const { meridiemMode, handleMeridiemChange } = useMeridiemMode<TDate>(
Expand Down Expand Up @@ -279,22 +282,18 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
],
);

const handleSectionChange = useEventCallback(
(sectionView: TimeViewWithMeridiem, newValue: TDate | null) => {
const viewIndex = views.indexOf(sectionView);
const nextView = views[viewIndex + 1];
setValueAndGoToView(newValue, nextView, sectionView);
},
);

const buildViewProps = React.useCallback(
(viewToBuild: TimeViewWithMeridiem): MultiSectionDigitalClockViewProps<any> => {
switch (viewToBuild) {
case 'hours': {
return {
onChange: (hours) => {
const valueWithMeridiem = convertValueToMeridiem(hours, meridiemMode, ampm);
handleSectionChange('hours', utils.setHours(valueOrReferenceDate, valueWithMeridiem));
setValueAndGoToNextView(
utils.setHours(valueOrReferenceDate, valueWithMeridiem),
'finish',
'hours',
);
},
items: getHourSectionOptions({
now,
Expand All @@ -311,7 +310,11 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
case 'minutes': {
return {
onChange: (minutes) => {
handleSectionChange('minutes', utils.setMinutes(valueOrReferenceDate, minutes));
setValueAndGoToNextView(
utils.setMinutes(valueOrReferenceDate, minutes),
'finish',
'minutes',
);
},
items: getTimeSectionOptions({
value: utils.getMinutes(valueOrReferenceDate),
Expand All @@ -328,7 +331,11 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
case 'seconds': {
return {
onChange: (seconds) => {
handleSectionChange('seconds', utils.setSeconds(valueOrReferenceDate, seconds));
setValueAndGoToNextView(
utils.setSeconds(valueOrReferenceDate, seconds),
'finish',
'seconds',
);
},
items: getTimeSectionOptions({
value: utils.getSeconds(valueOrReferenceDate),
Expand Down Expand Up @@ -380,7 +387,7 @@ export const MultiSectionDigitalClock = React.forwardRef(function MultiSectionDi
localeText.minutesClockNumberText,
localeText.secondsClockNumberText,
meridiemMode,
handleSectionChange,
setValueAndGoToNextView,
valueOrReferenceDate,
disabled,
isTimeDisabled,
Expand Down Expand Up @@ -504,8 +511,9 @@ MultiSectionDigitalClock.propTypes = {
minutesStep: PropTypes.number,
/**
* Callback fired when the value changes.
* @template TDate, TView
* @param {TDate | null} value The new value.
* @template TValue The value type. Will be either the same type as `value` or `null`. Can be in `[start, end]` format in case of range value.
* @template TView The view type. Will be one of date or time views.
* @param {TValue} value The new value.
* @param {PickerSelectionState | undefined} selectionState Indicates if the date selection is complete.
* @param {TView | undefined} selectedView Indicates the view in which the selection has been made.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export const MultiSectionDigitalClockSection = React.forwardRef(
) {
const containerRef = React.useRef<HTMLUListElement>(null);
const handleRef = useForkRef(ref, containerRef);
const previousSelected = React.useRef<HTMLElement | null>(null);
const previousActive = React.useRef<HTMLElement | null>(null);

const props = useThemeProps({
props: inProps,
Expand Down Expand Up @@ -160,21 +160,17 @@ export const MultiSectionDigitalClockSection = React.forwardRef(
if (containerRef.current === null) {
return;
}
const selectedItem = containerRef.current.querySelector<HTMLElement>(
const activeItem = containerRef.current.querySelector<HTMLElement>(
'[role="option"][aria-selected="true"]',
);
if (!selectedItem || previousSelected.current === selectedItem) {
// Handle setting the ref to null if the selected item is ever reset via UI
if (previousSelected.current !== selectedItem) {
previousSelected.current = selectedItem;
}
return;
if (active && autoFocus && activeItem) {
activeItem.focus();
}
previousSelected.current = selectedItem;
if (active && autoFocus) {
selectedItem.focus();
if (!activeItem || previousActive.current === activeItem) {
return;
}
const offsetTop = selectedItem.offsetTop;
previousActive.current = activeItem;
const offsetTop = activeItem.offsetTop;

// Subtracting the 4px of extra margin intended for the first visible section item
containerRef.current.scrollTop = offsetTop - 4;
Expand Down
5 changes: 3 additions & 2 deletions packages/x-date-pickers/src/TimeClock/TimeClock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,9 @@ TimeClock.propTypes = {
minutesStep: PropTypes.number,
/**
* Callback fired when the value changes.
* @template TDate, TView
* @param {TDate | null} value The new value.
* @template TValue The value type. Will be either the same type as `value` or `null`. Can be in `[start, end]` format in case of range value.
* @template TView The view type. Will be one of date or time views.
* @param {TValue} value The new value.
* @param {PickerSelectionState | undefined} selectionState Indicates if the date selection is complete.
* @param {TView | undefined} selectedView Indicates the view in which the selection has been made.
*/
Expand Down
Loading

0 comments on commit 0678e0a

Please sign in to comment.