Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
drmax24 opened this issue Nov 28, 2024 · 4 comments
Open

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

drmax24 opened this issue Nov 28, 2024 · 4 comments
Labels
bug 🐛 Something doesn't work component: date time picker This is the name of the generic UI component, not the React module! component: pickers This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes

Comments

@drmax24
Copy link

drmax24 commented Nov 28, 2024

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/[email protected]":
version "5.16.7"
next@^13.1.3:
version "13.1.6"

Search keywords: DateTimePicker onAccept

@drmax24 drmax24 added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 28, 2024
@michelengelen michelengelen changed the title If I click outside of the DateTimePicker popup onAccept is fired [pickers][DateTimePicker] Clicking outside of the popup fires onAccept Nov 28, 2024
@michelengelen michelengelen added component: pickers This is the name of the generic UI component, not the React module! component: date time picker This is the name of the generic UI component, not the React module! labels Nov 28, 2024
@michelengelen
Copy link
Member

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
Copy link
Member

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.

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 28, 2024
@drmax24
Copy link
Author

drmax24 commented Nov 28, 2024

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.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Nov 28, 2024
@michelengelen
Copy link
Member

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.

@michelengelen michelengelen added waiting for 👍 Waiting for upvotes and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: date time picker This is the name of the generic UI component, not the React module! component: pickers This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

3 participants