[pickers] Avoid stealing focus on click away #13434

LukasTy · 2024-06-10T10:42:17Z

Fix a related issue with Range Pickers: the stopPropagation calls were preventing click away listener from working properly
Fix a related issue with Pickers using Digital Clocks: The .focus() calls in effects were stealing the focus before closing the popper

Preview: https://deploy-preview-13434--material-ui-x.netlify.app/x/react-date-pickers/

mui-bot · 2024-06-10T10:45:27Z

Deploy preview: https://deploy-preview-13434--material-ui-x.netlify.app/

Bundle size report

Bundle	Parsed Size	Gzip Size
@mui/x-data-grid	0B^(0.00%)	0B^(0.00%)
@mui/x-data-grid/DataGrid	0B^(0.00%)	0B^(0.00%)
@mui/x-data-grid-pro	0B^(0.00%)	🔺+3B^(0.00%)
@mui/x-data-grid-pro/DataGridPro	0B^(0.00%)	🔺+4B^(0.00%)
@mui/x-data-grid-premium	0B^(0.00%)	🔺+4B^(0.00%)
@mui/x-data-grid-premium/DataGridPremium	0B^(0.00%)	🔺+4B^(0.00%)
@mui/x-charts	0B^(0.00%)	0B^(0.00%)
@mui/x-charts-pro	0B^(0.00%)	🔺+4B^(0.00%)
@mui/x-date-pickers	🔺+670B^(+0.29%)	🔺+316B^(+0.51%)
@mui/x-date-pickers-pro	🔺+670B^(+0.21%)	🔺+340B^(+0.41%)
@mui/x-tree-view	0B^(0.00%)	0B^(0.00%)
@mui/x-tree-view-pro	0B^(0.00%)	🔺+2B^(+0.01%)

Details of bundle changes

Generated by 🚫 dangerJS against 334bf65

…r-behavior

packages/x-date-pickers-pro/src/internals/hooks/useEnrichedRangePickerFieldProps.ts

LukasTy · 2024-06-10T12:38:11Z

packages/x-date-pickers/src/DigitalClock/DigitalClock.tsx

@@ -219,7 +220,8 @@ export const DigitalClock = React.forwardRef(function DigitalClock<TDate extends
      return;
    }
    const offsetTop = activeItem.offsetTop;
-    if (autoFocus || !!focusedView) {
+    if ((autoFocus || !!focusedView) && activeItem !== lastActiveRef.current) {


Fix the issue of stealing the focus when closing a picker with DigitalClock:

Screen.Recording.2024-06-10.at.15.37.51.mov

LukasTy · 2024-06-10T12:39:05Z

packages/x-date-pickers/src/MultiSectionDigitalClock/MultiSectionDigitalClockSection.tsx

@@ -172,7 +173,7 @@ export const MultiSectionDigitalClockSection = React.forwardRef(
      const activeItem = containerRef.current.querySelector<HTMLElement>(
        '[role="option"][tabindex="0"], [role="option"][aria-selected="true"]',
      );
-      if (active && autoFocus && activeItem) {
+      if (activeItem && active && autoFocus && activeItem !== previousActive.current) {


Fix the issue of stealing the focus when closing a picker with MultiSectionDigitalClock:

Screen.Recording.2024-06-10.at.15.38.47.mov

packages/x-date-pickers/src/internals/components/PickersPopper.tsx

michelengelen

LGTM! 🚢

flaviendelangle

Looks solid!

oliviertassinari

In HEAD, we restore the focus back to the trigger. e.g. https://mui.com/material-ui/react-menu/

Screen.Recording.2024-06-10.at.18.37.04.mov

It doesn't do it anymore:

Screen.Recording.2024-06-10.at.18.34.53.mov

A bug?

packages/x-date-pickers-pro/src/internals/hooks/useEnrichedRangePickerFieldProps.ts

test/e2e/index.test.ts

LukasTy · 2024-06-11T07:56:55Z

In HEAD, we restore the focus back to the trigger. e.g. https://mui.com/material-ui/react-menu/
It doesn't do it anymore:
A bug?

Thank you for flagging this @oliviertassinari! 🙏
I've used the behavior in here as an example.
There is a clear issue that closing the picker with an Esc key no longer returns the focus to the button as in the referred example.
Given the W3C-ARIA Menu example you could think that our Menu implementation is a bit too intrusive with moving the focus back to the button even on click away.
I'll look into possible options a bit more in-depth. 😉

LukasTy · 2024-06-11T13:28:09Z

@oliviertassinari I've explored other solutions and here is the rundown:

Angular Material and React Spectrum behave like we currently do (click away returns focus back to the opening button), but they both are using the dialog approach, where all the rest of the page becomes non-interactive—under a fullscreen backdrop, in our case—we don't do this.
Native date input element behaves like in this PR implementation in both Chrome and Firefox—Esc returns the focus to the opening button, but clicking away doesn't. That seems to be inline with W3C-ARIA example guidelines.
Next UI seems to have the same behavior like we currently do, no backdrop and proper click-away handling. If that's a role model we want to follow, when I need to "get back to the drawing board".

Given these arguments, I think that the behavioral solution in this PR is valid and more correct than the current one.
Unless we want to have a full-screen backdrop visually "disabling" page interactiveness, in which case we need more changes.

WDYT @flaviendelangle @arthurbalduini @joserodolfofreitas?

Do you think that adding a fullscreen backdrop makes sense in this case?
Or do you feel our approach closer resembling a native date input behavior makes sense to you?
Or do we want to strive for making no behavioral changes and just fixing the underlying problem—Next UI approach?

oliviertassinari · 2024-06-11T14:05:52Z

@LukasTy Thanks for the dive. In my view, the behavior before feels better (Angular Material and React Spectrum ). At least, the things that are important properties we want seem to be:

Tabs go to the next form input after closing a date picker, on all platforms
the screen readers announce the right focus element, it's not lost

On the solution, I don't think that 1. (using a backdrop) is that much correlated to the root of the problem, this feels like taking a different path (which works, true). Now, it's not the end of the world, maybe it can work ok like this.

At the root, there might be nothing to change in MUI X (I assume what we do here is to trigger ). The problem might really be with the FocusTrap logic. Maybe the logic that restores the focus should give up if that focus is already set outside of its boundary. I mean, this would make a lot of sense to me. It makes me think of mui/material-ui#35307. For this, we should involve the FocusTrap's ower? https://www.notion.so/mui-org/0c8156b0b1634ea5bfd903dd005ec1cf?v=89ea8d7b7e104bf8b3bb68a9edca5e16&p=fbe1aed295f748c19576d024e9a48c62&pm=s

jbblanchet · 2024-09-30T17:37:03Z

Hi! Is this fix still being considered? I just spent the better part of last week migrating from react-dates to this date picker, but that bug is a deal breaker for us. I haven't see any comments since June on this issue, I was wondering if maybe it had been forgotten?

Thanks

github-actions · 2024-10-10T11:45:06Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

LukasTy · 2024-10-10T12:34:48Z

Hi! Is this fix still being considered? I just spent the better part of last week migrating from react-dates to this date picker, but that bug is a deal breaker for us. I haven't see any comments since June on this issue, I was wondering if maybe it had been forgotten?

Hi, thank you for reminding us about this problem. 🙏
It's been parked, because of higher-priority topics for now, but we plan to finalize this PR in the very near future. 👌

BrunMartins · 2025-01-31T07:07:23Z

Hello! Was there any progress on this issue in the meanwhile? If not, can the community help in anyway?
Thanks!

github-actions · 2025-03-26T03:17:54Z

This pull request has been inactive for 30 days. Please remove the stale label or leave a comment to keep it open. Otherwise, it will be closed in 15 days.

github-actions · 2025-04-10T03:20:46Z

This pull request has been closed due to 15 days of inactivity after being marked stale.

…er interactive elements

LukasTy · 2025-07-11T12:55:05Z

I have given this a second shot.

Exploring the existing approaches in different libraries, I have noticed two main behavior options (as noted in #13434 (comment)):

Modal with backdrop approach (nothing is clickable outside of the dialog).
This is the easy-way–out, since it simply blocks the users from encountering issues, which would be possible if other elements in the page were clickable
A hybrid approach without a backdrop (which we currently have).
This works well on HeroUI (previously NextUI), hence I've looked into a solution (not perfect, but should cover 99% of cases).

I have gone for the 2nd approach as it's the least UX change and could be treated as a bugfix, without a BC.
The remaining issue is that HeroUI closes the dialog on scroll and I think it is necessary, because otherwise clicking away while being scrolled away from the target element results in scroll jumping to item that we returned to focus to.

I haven't looked into this improvement, since it is not strictly necessary and is a more obvious "breaking change", but I think it is necessary for the complete solution and could/should be done on the next major.

As for when to close the Picker on scroll - it's debatable. We could do it only when the Picker leaves visible viewport. IMHO, it's a bit more friendly UX than closing immediately after a tiniest amount of scroll, especially given our more complex Picker views (i.e. DateTimeRangePicker).

I'm open to suggestions and ideas on whether we should backport the fix to v7. 👍
Re-requesting reviews.

github-actions · 2025-07-28T10:58:04Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

flaviendelangle

Congrats (again) for the work on this one!
It's not an easy one to review, I tried in a few scenario and could not find issue 😬

flaviendelangle · 2025-08-26T15:42:01Z

packages/x-date-pickers-pro/src/DesktopDateRangePicker/tests/DesktopDateRangePicker.test.tsx

@@ -785,4 +785,25 @@ describe('<DesktopDateRangePicker />', () => {
      expect(getPickerDay('17')).to.have.attribute('disabled');
    });
  });
+  it('should close the Picker and move focus to the text field when clicking it', async () => {


Suggested change

it('should close the Picker and move focus to the text field when clicking it', async () => {

it('should close the Picker and move focus to the text field when clicking it', async () => {

flaviendelangle · 2025-08-26T15:43:40Z

packages/x-internals/src/isElementInteractive/isElementInteractive.ts

+ */
+const isInputHidden = (input: HTMLInputElement) => input.type === 'hidden';
+
+export function isElementInteractive(element: HTMLElement) {


If it's only used in one package, not sure if it make sense to extract it.
Is it likely that we end up using it elsewhere?

LukasTy added 4 commits June 4, 2024 18:26

[pickers] Fix to avoid refocusing previous item on click away

23f53da

Avoid stopPropagation to fix clickAway behavior on range picker

da055ed

Add all Picker components focus assertion tests

e8e1b9a

Avoid stealing focus when blurring digital clocks

bcc471d

LukasTy added type: bug It doesn't behave as expected. scope: pickers Changes related to the date/time pickers. needs cherry-pick The PR should be cherry-picked to master after merge. labels Jun 10, 2024

LukasTy self-assigned this Jun 10, 2024

Merge remote-tracking branch 'upstream/master' into fix-pickers-poppe…

176c30d

…r-behavior

LukasTy commented Jun 10, 2024

View reviewed changes

Use nodeName to make jest happy

56fc153

LukasTy requested review from oliviertassinari and a team June 10, 2024 13:26

michelengelen approved these changes Jun 10, 2024

View reviewed changes

flaviendelangle approved these changes Jun 10, 2024

View reviewed changes

oliviertassinari reviewed Jun 10, 2024

View reviewed changes

packages/x-date-pickers-pro/src/internals/hooks/useEnrichedRangePickerFieldProps.ts Outdated Show resolved Hide resolved

test/e2e/index.test.ts Outdated Show resolved Hide resolved

Remove preventDefault calls as they do not seem required

c393b79

michelengelen mentioned this pull request Jul 23, 2024

[pickers] Inconsistent design of DatePicker and DateRangePicker #13941

Closed

michelengelen mentioned this pull request Oct 2, 2024

[pickers] Dropdown Anchor Movement Issue in Date/Time Pickers #14801

Open

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 10, 2024

github-actions bot added the stale Inactive for 7 days (issues) or 30 days (PRs); closed after 5 or 15 more days if no update. label Mar 26, 2025

github-actions bot closed this Apr 10, 2025

LukasTy mentioned this pull request Apr 29, 2025

[pickers] Scroll behavior happening when having more than one DatePicker #17596

Closed

LukasTy added this to the Date and Time Pickers v8 post stable launch milestone May 23, 2025

LukasTy added 3 commits July 8, 2025 13:53

Merge branch 'master' into fix-pickers-popper-behavior

757bb59

Merge branch 'master' into fix-pickers-popper-behavior

56aba6a

Fix after merge

f022512

LukasTy reopened this Jul 9, 2025

github-actions bot removed the stale Inactive for 7 days (issues) or 30 days (PRs); closed after 5 or 15 more days if no update. label Jul 9, 2025

LukasTy added 2 commits July 10, 2025 15:04

Remove seemingly pointless prop

2ee9950

Ensure previous focused element is not refocused when clicking on oth…

4e1b5f9

…er interactive elements

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jul 10, 2025

LukasTy removed the needs cherry-pick The PR should be cherry-picked to master after merge. label Jul 11, 2025

LukasTy added 3 commits July 11, 2025 13:32

Replace e2e tests with unit

e56a307

Merge branch 'master' into fix-pickers-popper-behavior

f58e09b

Ensure dialog is closed before assertion

64d89d2

LukasTy requested review from oliviertassinari, michelengelen and flaviendelangle July 11, 2025 12:55

This was referenced Jul 15, 2025

[pickers] Fix popper click-away behavior #18804

Merged

[pickers] Lose focus of an input when re-rendering a static time picker #18356

Open

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jul 28, 2025

Merge branch 'master' into fix-pickers-popper-behavior

f2e4069

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jul 29, 2025

LukasTy added 2 commits July 31, 2025 17:23

Merge branch 'master' into fix-pickers-popper-behavior

a180bb5

Try full manual focus control

334bf65

flaviendelangle approved these changes Aug 26, 2025

View reviewed changes

	it('should close the Picker and move focus to the text field when clicking it', async () => {

	it('should close the Picker and move focus to the text field when clicking it', async () => {

Uh oh!

[pickers] Avoid stealing focus on click away #13434

Are you sure you want to change the base?

[pickers] Avoid stealing focus on click away #13434

Uh oh!

Conversation

LukasTy commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mui-bot commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report

Uh oh!

Uh oh!

LukasTy Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

LukasTy Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michelengelen left a comment

Choose a reason for hiding this comment

Uh oh!

flaviendelangle left a comment

Choose a reason for hiding this comment

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LukasTy commented Jun 11, 2024

Uh oh!

LukasTy commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbblanchet commented Sep 30, 2024

Uh oh!

github-actions bot commented Oct 10, 2024

Uh oh!

LukasTy commented Oct 10, 2024

Uh oh!

BrunMartins commented Jan 31, 2025

Uh oh!

github-actions bot commented Mar 26, 2025

Uh oh!

github-actions bot commented Apr 10, 2025

Uh oh!

LukasTy commented Jul 11, 2025

Uh oh!

github-actions bot commented Jul 28, 2025

Uh oh!

flaviendelangle left a comment

Choose a reason for hiding this comment

Uh oh!

flaviendelangle Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

flaviendelangle Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LukasTy commented Jun 10, 2024 •

edited

Loading

mui-bot commented Jun 10, 2024 •

edited

Loading

LukasTy commented Jun 11, 2024 •

edited

Loading

oliviertassinari commented Jun 11, 2024 •

edited

Loading