react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

Clicking outside the DatePicker can manipulate the focus of the DateSegment. (react-aria's DatePicker)

Open RyoSogawa opened this issue 2 years ago โ€ข 11 comments

๐Ÿ› Bug Report

Clicking outside the DatePicker can manipulate the focus of the DateSegment.

react-spectrum's DatePicker does not cause this phenomenon. Only react-aria's DatePicker.

๐Ÿค” Expected Behavior

Clicking outside of DatePicker does not focus or move focus.

๐Ÿ˜ฏ Current Behavior

Image from Gyazo

๐Ÿ’ Possible Solution

๐Ÿ”ฆ Context

๐Ÿ’ป Code Sample

https://react-spectrum.adobe.com/react-aria/useDatePicker.html#example

๐ŸŒ Your Environment

Software Version(s)
@react-stately/datepicker ~3.0.0-rc.0
@react-aria/datepicker ~3.0.0-rc.0
Browser Chrome
Operating System macOS

๐Ÿงข Your Company/Team

๐Ÿ•ท Tracking Issue (optional)

RyoSogawa avatar May 26 '22 04:05 RyoSogawa

Thanks for filing this issue, not exactly sure what is going on here. As you noted, this isn't happening for our RSP DatePicker nor for the styled examples in the aria docs so perhaps there is some styling specific things happening in those components that prevent this issue

LFDanLu avatar Jun 01 '22 20:06 LFDanLu

Moving my comments here

Looks like it's related to contentEditable https://stackoverflow.com/questions/24345805/prevent-contenteditable-element-from-getting-focus-when-clicking-parent

https://codesandbox.io/s/silent-snowflake-nzv9xf?file=/src/App.js

That's without any React Spectrum or React Aria. Content editable fields are focusable when you click an ancestor in Chrome. Chrome is also the one responsible for figuring out how close you clicked to any given editable field, which is why you can change which one is currently active.

We could add a line to our docs to prevent this behavior, just a user-select: none on the parent, like here https://codesandbox.io/s/heuristic-dubinsky-bcnnrd

Here's a more in depth answer as to why it happens btw https://stackoverflow.com/questions/34354085/clicking-outside-a-contenteditable-div-stills-give-focus-to-it

snowystinger avatar Jun 08 '22 20:06 snowystinger

Bringing this back up. Looks like something else is causing this now. I just checked out the codesandbox for the fix to add user-select: none; to the parent. The fix didn't actually work and the docs and provided sandbox both appear to have the same issue still. Any possibly way we can investigate this again?

jake-chapman-mark43 avatar Nov 01 '22 05:11 jake-chapman-mark43

@jake-chapman-mark43 it's interesting, when I opened the codesandbox the first time, it was missing the userSelect none. However, the second time I opened it, it had it. Can you try one more time? With the userSelect none, I'm seeing the correct behavior. I've added a border to make it easier to see the previously troublesome area

snowystinger avatar Nov 01 '22 18:11 snowystinger

@snowystinger the user-select:none approach doesn't work well on safari. it fixes the focus issue but then you can't type into the segments. see forked version in safari (note I had to use webkitUserSelect: "none" for safari support) : https://codesandbox.io/s/smoosh-wind-lg2rqb?file=/src/App.js

[edit] I attempted to fix this behaviour on safari by resetting the user-select on the indiviaul segments but that then breaks the focus fix: https://codesandbox.io/s/young-wood-80qlxi?file=/src/App.js

[edit2] Workaround for safari: https://codesandbox.io/s/smoosh-wind-9rx6pt?file=/src/App.js

badsyntax avatar Apr 25 '23 07:04 badsyntax

Note this is bug DOES affect react-spectrum's DatePicker, but it's only evident on certain examples that use display: flex on their parent:

https://user-images.githubusercontent.com/102141/234239941-397a9f79-fc20-4b07-a67c-33f06cb2e9ba.mov

Play with the examples in the Granularity section to see this behaviour.

So it seems like display: flex affects this behaviour.

Here's a new codepen that shows using display: inline-flex fixes it for both chrome and safari: https://codesandbox.io/s/hopeful-pare-xt775d?file=/src/App.js

In that codepen, if you change it to display: flex you can see the focus bug again.

badsyntax avatar Apr 25 '23 09:04 badsyntax

Thanks for the sandbox. Putting user-select: none; is working for me in the sandbox. And thanks for the React Spectrum tip, it looks like the same thing needs to be applied there. I'm trying to see if there is a place we could apply it either through our hook props or at least in our component's rendering, however, it seems not to matter if it's an immediate parent or not. As you can see here I've added a parent again with padding and even though the earlier problematic one has the user-select trick, the new ancestor would also need it https://codesandbox.io/s/summer-pine-q8xbh3?file=/src/App.js

It does look like one of the other workarounds in the StackOverflow holds promise even with the additional ancestor. It does need to be used in conjunction with the first workaround though from what I can tell. Here's an example https://codesandbox.io/s/broken-rain-c5lsl0?file=/src/App.js

Unfortunately, we cannot build the invisible characters into the hooks. So that part will need to be done by you. But we might be able to add the user-select through the hook useDateField's fieldProps. We could also come up with a convenience component for wrapping a content-editable in zero-width characters.

We welcome contributions if anyone wants to work on this.

snowystinger avatar Apr 25 '23 16:04 snowystinger

@snowystinger does user-select: none; work for you in Safari?

Safari using un-prefixed user-select: none rule (focus bug):

https://user-images.githubusercontent.com/102141/234502080-61e5a733-6344-4d9e-a681-755038de3824.mov

Safari using webkit prefixed -wekit-user-select: none (no focus bug, but unable to edit text):

https://user-images.githubusercontent.com/102141/234502126-989b101c-f610-4d53-acf2-26430d2ed877.mov

badsyntax avatar Apr 26 '23 07:04 badsyntax

The second example i shared is working for me in Safari without the prefix. Safari 16.4

snowystinger avatar Apr 26 '23 16:04 snowystinger

Reading @snowystinger detailed link to stackoverflow article.

The cleanest solution is workaround with div wrapper. This can be done since DateInput expects render function anyway. So just wrap DateSegment with empty div.

<DateInput className={theme.dateInput} slot="end">
	{segment => (
		<div>
			<DateSegment className={theme.dateSegment} segment={segment} />
		</div>
	)}
</DateInput>

Example: https://codesandbox.io/p/sandbox/small-cloud-qd67sx

Unfortunatelly, it still focus date separators like '/'.

Edit:

It seems to work in Firefox and Edge, if I combine with wrapper div having user-select: none. Need to check Safari

mauron85 avatar Mar 09 '24 08:03 mauron85

I had a similar problem with the datepicker. Clicking on any input field (i.e. month or day) would still make it jump to the year field. I could navigate back though through Option + Shift. I tried using user-select: none but this didn't work.

What did help was getting rid of a Group that I had around the DateInput and a calendar Button. I simply replaced it by a div and it seems to work now. Maybe this helps somebody else.

bielern avatar Apr 22 '24 18:04 bielern

Faced the same issue. It is very annoying, because breaks any logic which may depend on focused state of the field/segments. I tried to dynamically set contentEditable, and it fixed focus, but not blur - when click exactly below the segment, it focuses the segment.

But looks like I found the solution, at least it works for me. Just wrap the segment into span, the segment should be div without any display property (all kinds of flex and grid breaks it). The opposite wrapping div>span doesn't work

Here is the example

EliteUser avatar May 23 '24 10:05 EliteUser

I'm encountering the same issue on my project, bumping this, hoping for a fix soon :)

CarlosVergikosk avatar Aug 05 '24 10:08 CarlosVergikosk