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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DatePicker] shouldDisableDate runs on all dates ever, not just visible ones before opening #4705

Closed
2 tasks done
DanielAndrews43 opened this issue Jan 31, 2022 · 15 comments
Closed
2 tasks done
Labels
component: DatePicker The React component. component: pickers This is the name of the generic UI component, not the React module! performance

Comments

@DanielAndrews43
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

When using DesktopDatePicker it runs shouldDisableDate on 100s of years of dates when first opening the modal. This delays the modal opening for several seconds. When switching between months, it only runs shouldDisableDate on the visible dates, which looks correct.

  • Broken on first open
  • Works great once it's already open
Screen.Recording.2022-01-31.at.12.15.24.PM.mov

Expected behavior 馃

It should only run shouldDisableDate on the visible dates

Steps to reproduce 馃暪

Steps:

  1. Create a DesktopDatePicker component with shouldDisableDate prop
  2. Click on the calendar icon to open the date picker modal
<LocalizationProvider dateAdapter={AdapterDateFns}>
  <DatePicker
    label="Basic example"
    value={value}
    onChange={(newValue) => {
      setValue(newValue);
    }}
    shouldDisableDate={() => false} // this function will run on every date.
    renderInput={(params) => <TextField {...params} />}
  />
</LocalizationProvider>

Context 馃敠

We are creating invoicing software and only want dates to be selectable as a due date if they are on the correct billing day. This can be an expensive function for us to run if we need to pull additional data about exceptions. If it only runs on visible days there is almost no delay visible to the user, but if it's running on all historical dates then it ends up in 100s of network requests and slows down everything.

Your environment 馃寧

`npx @mui/envinfo`
 System:
    OS: macOS 12.1
  Binaries:
    Node: 14.1.0 - ~/.nvm/versions/node/v14.1.0/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v14.1.0/bin/npm
  Browsers:
    Chrome: 97.0.4692.99
    Edge: Not Found
    Firefox: 57.0.4
    Safari: 15.2
  npmPackages:
    @emotion/react: ^11.4.1 => 11.7.1 
    @emotion/styled: ^11.3.0 => 11.3.0 
    @mui/base:  5.0.0-alpha.66 
    @mui/core:  5.0.0-alpha.50 
    @mui/lab: ^5.0.0-alpha.66 => 5.0.0-alpha.66 
    @mui/material: ^5.0.3 => 5.0.3 
    @mui/private-theming:  5.0.1 
    @mui/styled-engine:  5.0.1 
    @mui/system:  5.0.3 
    @mui/types: ^7.0.0 => 7.0.0 
    @mui/utils:  5.0.1 
    @types/react:  17.0.11 
    react: 17.0.0 => 17.0.0 
    react-dom: 17.0.0 => 17.0.0 
    styled-components: ^4.4.0 => 4.4.1 
    typescript: ^4.1.3 => 4.3.4 
@DanielAndrews43 DanielAndrews43 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 31, 2022
@danilo-leal danilo-leal added component: DatePicker The React component. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 4, 2022
@gl-aagostino
Copy link

+1, makes over 28k calls to that function on load.
Also if this were to be fixed, would it be put into MUI 4.x ?

@apenab
Copy link

apenab commented Feb 23, 2022

I'm having the same problem.

@flaviendelangle flaviendelangle transferred this issue from mui/material-ui Apr 29, 2022
@flaviendelangle flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label Apr 29, 2022
@flaviendelangle
Copy link
Member

@apenab @DanielAndrews43 @gl-aagostino
Could you give me a reproduction case on Codesandbox ?
I tried a few things but I always have 35-50 calls to shouldDisableDate

This template could be a good starting point

@Dragomir-Ivanov
Copy link

@flaviendelangle
I am also having this problem, however when I use your sandbox it doesn't happen. I did set my versions of "@mui/*" in that sandbox and still can't reproduce. Any advice how to proceed appreciated.

@flaviendelangle
Copy link
Member

Without a reproduction case it's hard to to a diagnostic.
Are you passing the same props to our component both in your application and in the codesandbox ?

@Dragomir-Ivanov
Copy link

@flaviendelangle Yes, I am trying to. Can we trust CodeSandbox, that it really uses downgraded dependencies, and not some leftovers?

@flaviendelangle
Copy link
Member

The version should be respected yes
Can you link me your codesandbox ?

If you successfully reproduce it locally on an empty repo (with CRA or Next.JS for instance) I can clone it and test it on my machine as well.
Codesandbox is of course a lot easier but if it does not work.

@Dragomir-Ivanov
Copy link

@flaviendelangle
Thank you, I will try make a separate project. Could it be Vite.js issue, since I am using Vite.
Stay tuned.

@Dragomir-Ivanov
Copy link

Dragomir-Ivanov commented Jul 8, 2022

@flaviendelangle
I think I succeeded reproducing this. Can you try: https://codesandbox.io/s/date-and-time-pickers-forked-3nelms

Latest versions of all @mui/* libraries.
Note that shouldDisableDate always returns true, if you make it return false it works as expected.

@Dragomir-Ivanov
Copy link

Dragomir-Ivanov commented Jul 8, 2022

@flaviendelangle Here is another codesandbox: https://codesandbox.io/s/date-and-time-pickers-forked-pqlc5j

This actually uses your template. Changes are:

  1. Provide initialized value.
  2. Make all dates disabled.

@Dragomir-Ivanov
Copy link

Dragomir-Ivanov commented Jul 14, 2022

Hi @flaviendelangle
I have reproduced the issue in codesandbox. Do you managed to see it? Can I help with anything else?

@alexfauquette
Copy link
Member

This does not look like a bug. When no date is open, the component is looking for the closest valid date. You can verify this by returning.

Here is an example with only one valid date (the 5 June 2020) And the component will loop over days until it find it. After that the onChange is called to set the valid date.
https://codesandbox.io/s/date-and-time-pickers-forked-ry20f7?file=/src/App.tsx:578-596

If you have not available date, you should disable the component. If you know the first available date, setting it will prevent this search.

@flaviendelangle I think you already fixed this in #6146

@flaviendelangle
Copy link
Member

Yep, #6146 should close the problem 馃憤

@sheppard30
Copy link

@flaviendelangle when we can expect release?

@flaviendelangle
Copy link
Member

The fix has only been done on the v6 alpha.
But it should definitely be back-porter to the v5 releases, I'll do it 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: DatePicker The React component. component: pickers This is the name of the generic UI component, not the React module! performance
Projects
None yet
Development

No branches or pull requests

8 participants