Desktop pickers don't work inside modals

[thaind97git]

https://codesandbox.io/s/range-picker-issue-f9g99?file=/index.js

Environment

Tech	Version
@material-ui/pickers	v4.0.0-alpha.7
material-ui	4.10.0
TypeScript	Javascript
React
Browser
Peer library	moment 2.6.0

Steps to reproduce

1. Setup a DateRangePickers inside a dialog Material-UI
2. Click the input field to open DateRange
3. Click to leftArrowIcon or rightArrowIcon. Then, the DateRange auto close.

Expected behavior

Click to leftArrowIcon or rightArrowIcon. Then, the DateRange will NOT auto close.

[dmtrKovalenko]

Thanks for rising an issue! But it is not enough information for us to properly reproduce and understand your problem. Can you please provide more details about the issue?

Please provide a reproduction, this codesandbox template could be useful.

Thanks ;)

[thaind97git]

@dmtrKovalenko , Thank you for your response. Here is code sandbox demo. When I click leftArrowIcon or rightArrowIcon, the expected result is wrong ( The DateRange auto close)

[oliviertassinari]

I'm happy to see this issue, it means the package starts to get more usage, and especially the next version.

@thaind97git It's a duplicate of https://github.com/mui-org/material-ui/issues/15694. We have been aware of this problem since the initial design of the date range picker, and it will soon impact the date picker with #1850.

Soon or later, the problem will be solved. I think that we can keep this issue open as a helpful reminder, we want to get to it, not only for the impact on the date pickers.

[oliviertassinari]

Duplicate of #1777 too.

[jotatoledo]

@oliviertassinari is there a way to work around this? Im currently experiencing the issue (v4.0.0-alpha.8) having:

Dialog>DateRangePicker

Once the user clicks any area of the picker-popover (eg.: left/right arrow), the picker is closed.

Any help would be appreciated.

[mikomarrache]

Is there a workaround that we can use for now?

Thanks

[oliviertassinari]

I can confirm that https://github.com/mui-org/material-ui/pull/21610 solves the issue. Proof: https://codesandbox.io/s/range-picker-issue-f9g99?file=/index.js.

@dmtrKovalenko @PaulSavignano What about we upgrade the peer dependency on @material-ui/core/v5.0.0-alpha.2 (with the trap focus fixes coming, there are 3 to solve) in the next release of the date picker? We could also update @material-ui/pickers version to v5.0.0-alpha.1 and keep iterating on it until we release a stable v5.

My experience so far has been that the more fixes we make with the alpha version of the picker, the more usage we get, the more issues we found that require breaking changes. I think that we should be "long" on a stable version, iterate on it under an unstable flag for the next 6/9 months with the same release schedule as the core, exactly like we did with the Autocomplete. It takes time to make great software, it seems to be too early to make a stable version, we are not ready yet. I think that we should do such once we are confident we have the best date picker in the React community, it's not the case yet, but soon.

Also, I think that we can start working on https://next.material-ui.com/components/pickers/.

[oliviertassinari]

Fix merged and will be released as @material-ui/core@5.0.0-alpha.2 this weekend.

[ggascoigne]

Is there any chance that that fix could be back ported to the 4x line? I'd love to continue using the material-pickers but we're not ready to switch the whole ui over the 5 until it's closer to release, having one component being alpha seems like the sort of thing where you can test the edge cases well enough to know where you are, having the whole ui be alpha isn't something that we can do right now.

[oliviertassinari]

@ggascoigne v4.x only accepts security patches and important bug fixes: e.g. a transitive dependency breaks the build. This won't be backported. Also, please mind that the date range picker you rely on as an alpha will be paid once stable, we have added a warning in the documentation (will soon be released).

[ggascoigne]

Understood, I understand why but I had to ask, for now we'll just have to stop using the DateRangePicker and try again when things are more stable.

I should point out though since you wrote this:

the more usage we get, the more issues we found that require breaking changes

If pickers becomes dependent on v5, you'll get a pretty substantial drop in usage until after v5 is both released and people have upgraded.

[ggascoigne]

hah, both editing our comments at the same time - sorry about that.

I'll add that I'm not adverse to paying for the feature, we've supported other open source projects so depending on the setup, we are certainly open to doing so here.

[oliviertassinari]

If pickers becomes dependent on v5, you'll get a pretty substantial drop in usage until after v5 is both released and people have upgraded.

@ggascoigne Don't worry about it, we have tried this patch in the past (v1, lab), it works pretty well, arguably, it's more efficient 👌. Once you get people begging for going stable, you know if you have hit something. Until you don't, you are not ready, you need to keep iterating. In our case, I don't think that we are ready.

I'll add that I'm not adverse to paying for the feature

This will part of a whole suite of enterprise components. The end goal is that you will never have to look outside for advanced features, data grid, charts, big calendar, etc. They will be two pricing tiers, one approachable for indie developers for features you would use a third-party open-source library instead (the value proposition for you being you could keep a single vendor + higher quality and consistency) and a higher tier, x10 more expensive for features no OSS alternatives exist (and probably never will because of the complexity & wrong incentives of OSS).

[oliviertassinari]

Alright, so the fix of this problem should be in this order:

diff --git a/docs/package.json b/docs/package.json
index 79693120..aa24610a 100644
--- a/docs/package.json
+++ b/docs/package.json
@@ -21,7 +21,7 @@
     "@date-io/hijri": "^2.6.0",
     "@date-io/jalaali": "^2.6.0",
     "@mapbox/rehype-prism": "^0.4.0",
-    "@material-ui/core": "^4.9.14",
+    "@material-ui/core": "^5.0.0-alpha.2",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "^4.0.0-alpha.54",
     "@material-ui/pickers": "^4.0.0-alpha.1",
diff --git a/lib/package.json b/lib/package.json
index be0656dc..d2a40f5b 100644
--- a/lib/package.json
+++ b/lib/package.json
@@ -32,7 +32,7 @@
     "email": "dmtr.kovalenko@outlook.com"
   },
   "peerDependencies": {
-    "@material-ui/core": "^4.10.1",
+    "@material-ui/core": "^5.0.0-alpha.2",
     "@material-ui/lab": "^4.0.0-alpha.54",
     "@types/react": "^16.8.6",
     "react": "^16.8.4",
@@ -82,7 +82,7 @@
     "@babel/preset-react": "^7.9.4",
     "@babel/runtime": "^7.9.6",
     "@cypress/webpack-preprocessor": "^4.1.1",
-    "@material-ui/core": "^4.9.14",
+    "@material-ui/core": "^5.0.0-alpha.2",
     "@material-ui/icons": "^4.9.1",
     "@rollup/plugin-babel": "^5.0.3",
     "@rollup/plugin-commonjs": "^12.0.0",

[BowlingX]

@oliviertassinari upgrading to ^5.0.0-alpha.2 or adding disableEnforceFocus in 4.x fixes the issue and the DateRange picker closes. Unfortunately, this seems not to work work with the "normal" pickers (see my sandbox here: https://codesandbox.io/s/bold-kare-0mcpc). The Picker does not close when I click somewhere else.

[oliviertassinari]

@BowlingX What issue did you observe on ^5.0.0-alpha.2?

[BowlingX]

The Picker won't close if it's inside a Modal and you click somewhere else to close it again. (see linked sandbox)

[oliviertassinari]

@BowlingX Thanks for detailing it. I can reproduce the problem. I'm not familiar with the close logic of the popup, we would need to look into this.

[BowlingX]

As a workaround, for the v4 material-ui version and disableEnforceFocus set in the Modal / Drawer, you can manually blur the activeElement on click in the Modal's tree like this:

/* eslint-disable no-extra-semi */
import { MouseEvent as ReactMouseEvent } from 'react'

export const hasParentWithClass = <T extends HTMLElement>(el: T, classname: string) => {
  let next: T | null = el.parentElement as T

  while (next !== null) {
    if (next.classList && next.classList.contains(classname)) {
      return true
    }
    next = next.parentElement as T | null
  }

  return false
}

export function handlePickersModalClick<T extends HTMLElement>(e: ReactMouseEvent<T, MouseEvent>) {
  if (!hasParentWithClass(e.target as T, 'MuiPickersPopper-paper')) {
    if (
      document.activeElement &&
      document.activeElement.classList.contains('MuiPickersPopper-paper')
    ) {
      ;(document.activeElement as T).blur()
    }
  }
}

const Cmp = () => {
   return (
     <Dialog>
        <DialogContent onClick={handlePickersModalClick}>
           <DatePicker />
        <DialogContent/>
     </Dialog>
  )
}

The problem is, that the activeElement is still the DatePicker itself, due to the FocusTrap. See https://github.com/mui-org/material-ui-pickers/blob/next/lib/src/_shared/PickersPopper.tsx#L99

I think this has to be fixed in the FocusTrap component, to properly focus the parent modal again.

[abdel]

I'm having a similar issue that was reported with DatePicker on "@material-ui/core": "^5.0.0-alpha.16" and "@material-ui/pickers": "4.0.0-alpha.11".

In previous comments, the commentators stated that upgrading to alpha resolved it, but that doesn't seem to be the case here with DatePicker. The error is triggered upon selecting any date in the date picker.

Any ideas if there are specific solutions to this?

[Error] RangeError: Maximum call stack size exceeded.
    focus
    contain (Unstable_TrapFocus.js:137)
    focus
    contain (Unstable_TrapFocus.js:137)
..................
    focus
    contain (Unstable_TrapFocus.js:137)

[oliviertassinari]

The biggest chunk of the problem was solved https://codesandbox.io/s/range-picker-issue-f9g99?file=/index.js. If you face an issue with v5.0.0-alpha.16, please open an issue in https://github.com/mui-org/material-ui with a reproduction.