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

crossFrame={false} has cross-browser Safari issues #249

Closed
cee-chen opened this issue Jul 6, 2023 · 7 comments
Closed

crossFrame={false} has cross-browser Safari issues #249

cee-chen opened this issue Jul 6, 2023 · 7 comments
Assignees
Labels

Comments

@cee-chen
Copy link

cee-chen commented Jul 6, 2023

Not sure if this is just me (on Safari v16.5.1), would love it if people could confirm if they can repro!

Reproducible CodeSandbox (using react-focus-on, but it should in theory be the same for react-focus-lock?):

@theKashey
Copy link
Owner

bug confirmed 😭

@cee-chen
Copy link
Author

cee-chen commented Jul 7, 2023

Thanks for the quick repro Anton!

Not sure if this is helpful or not, but @tkajtoch and I were testing crossFrame in react-focus-on locally in our codebase's Storybook, and were also getting that same behavior as well even in Chrome and FF and couldn't figure out why or what was happening (code was almost identical to the above CodeSandbox). We eventually figured out it was the setTimeout 0 in onWindowBlur race condition:

https://github.com/theKashey/react-focus-lock/blob/master/src/Trap.js#L185-L187

We changed the above setTimout duration from 0 to 1 and the crossFrame behavior started working for us locally. Maybe give it a shot on your end and see if it fixes Safari behavior as well? 🤞

@theKashey
Copy link
Owner

😲 onBlur is executed "a little later" using a deferAction helper of mine - https://github.com/theKashey/react-focus-lock/blob/master/src/util.js#L7
It works exactly via setTimeout(..,1) - you have found the root case.
Well, it uses setTimeout only in the absence of setImmediate, which may or may not be polyfilled by a bundler.

@cee-chen
Copy link
Author

cee-chen commented Jul 9, 2023

Oooh! It all makes sense now 🤦 All the credit goes to @tkajtoch for figuring out that changing setTimeout fixed the behavior!

Would changing onWindowBlur to use deferAction be a clean solution, or would just changing the 0 to a 1 on its timeout suffice? 🤔

@theKashey
Copy link
Owner

deferAction is a way to go. Tested in Chrome and safari - works great.

theKashey added a commit that referenced this issue Jul 9, 2023
fix: correct crossframe behavior for Safari, fixes #249
@theKashey
Copy link
Owner

Released as v2.9.5.

Change is not yet propagated to https://github.com/theKashey/react-focus-on, please help me understand theKashey/react-focus-on#74 (comment) first

@cee-chen
Copy link
Author

I think @tkajtoch is working on that - will keep you posted! Thanks as always for being amazingly responsive and speedy Anton!

nickspaargaren pushed a commit to nickspaargaren/react-focus-lock that referenced this issue Dec 11, 2024
nickspaargaren pushed a commit to nickspaargaren/react-focus-lock that referenced this issue Dec 11, 2024
fix: correct crossframe behavior for Safari, fixes theKashey#249
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants