mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.57k stars 1.35k forks source link

[pickers][DateTimePicker] Clicking outside of the popup fires `onAccept` #15646

Open drmax24 opened 5 days ago

drmax24 commented 5 days ago

Steps to reproduce

If I click outside of the DateTimePicker popup onAccept is fired

I want onAccept to be fired only if I press OK. As far as I understand, this should be an expected behavior. If I add a cancel button, though, and if I click cancel, then onAccept is not fired, which is good.

import dayjs from "dayjs";
import { LocalizationProvider } from "@mui/x-date-pickers/LocalizationProvider";
import { AdapterDayjs } from "@mui/x-date-pickers/AdapterDayjs";
import { DateTimePicker as BaseDateTimePicker } from "@mui/x-date-pickers/DateTimePicker";
import "dayjs/locale/de";
import { useState } from "react";
import { DateTimePickerSlotsComponentsProps } from "@mui/x-date-pickers/DateTimePicker/DateTimePicker.types";

export const DateTimePicker = ({
    label,
    value,
    onChange,
    onAccept,
    minDateTime,
    disablePast = true,
    slotProps: slotProps,
}: {
    label: string;
    value: string | undefined;
    onChange?: (value: dayjs.Dayjs | null) => void;
    onAccept?: (value: dayjs.Dayjs | null) => void;
    required?: boolean;
    minDateTime?: string | null;
    disablePast?: boolean;
    slotProps?: DateTimePickerSlotsComponentsProps<dayjs.Dayjs>;
}) => {
    const [tempDate, setTempDate] = useState(value ? dayjs(value) : null);

    const mergedSlotProps = {
        ...{
            toolbar: {
                hidden: true,
            },
            textField: {
                sx: {
                    "& .MuiInputBase-input": {
                        padding: "8px",
                    },
                },
            },
        },
        ...slotProps,
    };

    return (
        <LocalizationProvider dateAdapter={AdapterDayjs} adapterLocale="de">
            <BaseDateTimePicker
                label={label}
                defaultValue={tempDate}
                onChange={onChange}
                onAccept={(newValue) => {
                    if (newValue) {
                        setTempDate(newValue);
                        onAccept?.(newValue);
                    }
                }}
                ampm={false}
                disablePast={disablePast}
                minDateTime={minDateTime ? dayjs(minDateTime) : undefined}
                slotProps={mergedSlotProps}
            />
        </LocalizationProvider>
    );
};

Current behavior

If I click outside of the date popup onAccept is fired. Closing window is not a confirmation...

Expected behavior

I want onAccept to be fired only if I press OK. As far as I understand, this should be an expected behavior. Or at least this must be achievable.

Context

No response

Your environment

| Tech | Version | "@mui/x-date-pickers@^6": version "6.20.2" "@mui/material@5.16.7": version "5.16.7" next@^13.1.3: version "13.1.6"

Search keywords: DateTimePicker onAccept

michelengelen commented 5 days ago

I cannot find anything in the aria requirements for the dialog closing "on click away"-behavior. It makes sense though that when a new value is selected it will fire it. Otherwise you would need to reset the previous value in the field, which is arguably worse.

@LukasTy or @flaviendelangle do you remember the reasoning behind the implementation?

flaviendelangle commented 5 days ago

Most of the current lifecycle comes from #4408

The optimal behavior is quite subjective and can vary depending on the picker (does it has several view that are rendered one after another? does it have several views rendered side by side? or does it have a single view).

We settled for always comitting when dimissing the picker. This is a behavior we might want to make more customizable in the future, but there is nothing concrete planned for now. And as @michelengelen pointed out, resetting when clicking away comes with problems as well.

drmax24 commented 4 days ago

Just from the business side. There is a relatively important date picker. A person clicks on it, and the person forgets the old value, so the person intends to close the window without saving. But what I'm saying there is a use case for it.

michelengelen commented 4 days ago

Just from the business side. There is a relatively important date picker. A person clicks on it, and the person forgets the old value, so the person intends to close the window without saving. But what I'm saying there is a use case for it.

There are a few options we could take. One being implementing a flag that resets the value when clicked outside instead of accepting it. But as mentioned by @flaviendelangle we have nothing planned as of now.

I'll add a waiting for 👍🏼 label for now and add it to the board.