-
Notifications
You must be signed in to change notification settings - Fork 67
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
Clickable elements below the fold in Safari #79
Comments
And exactly this moment is not working. Let's try to find out why |
It seems to me - this is a pure Safari issue - https://codesandbox.io/s/responsive-navigation-safari-bug-jjf1m - clicking on the button does not select it, it focuses on the parent div. |
https://codesandbox.io/s/responsive-navigation-safari-bug-jjf1m - for now the only way to fix Safari behaviour is to apply focus by yourself // help browser to focus element you just clicked
document.addEventListener(
"click",
event => {
event.target.focus();
},
true // to be confirmed, probably overpower.
); |
Ok, here is documentation - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus Does clicking on a give it focus?No (even with a tabindex) Does tapping on a give it focus?No (even with a tabindex) So the only ways to fix the problem (for now) are:
So - look like the only valid way, but still not quite cool, is |
@theKashey Thank you so much for the quick response! Approach #2 seems like the most reasonable solution. I'll give it a shot 😄 |
You know - try to combine .2 with that "click-focus" hack I've posted above. You still need to focus the element somehow, or background element will retain the focus. |
I had the same issue and solved it this way (found this solution by someone else):
|
@theKashey Is there a way to use the "whitelist" approach in react-focus-on? It doesn't look like |
I am not sure it's the right solution for a problem. Actually, I am thinking about a better approach: // help browser to focus element you just clicked
document.addEventListener(
"focus",
event => {
if(eventTargetContainsFocusLock) {
event.preventDefault(); // don't let focus to be set "trought" focus lock.
}
},
true,
); In the same time - there is already such logic - https://github.com/theKashey/react-focus-lock/blob/master/src/Trap.js#L121-L159 The problem: without whitelist
with white list
what shall be done
where? Or what is causing the initial issue
So - the real problem is assumtion that the "focus order" does not include your own parent. And it does not almost in all cases, except I think this is what shall be changed. |
Ok ^^^ that did not work. I could not preserve "focused elements" as long as they are not focusable in Safari. But I've found a solution.
<div
// remove mouse down propagation from the lock
onMouseDown={e => e.preventDefault()}
// restore just lost "default" behaviour
onClick={e => e.target.focus()}
>
<FocusLock>
.... I would gently ask you to test this solution:
|
Clicking on links below the fold works with this approach! But now tabbing and hitting enter/space doesn't work on any browser 😢 Any ideas how to fix that? |
Please double check - running the provided example in Chrome and Safari - everything works as it should. |
Oof, my bad. I was expecting the Your solution works great. Thank you! |
Let's keep it open unless it's properly documented, or even implemented on the focus lock side. |
https://zellwk.com/blog/inconsistent-button-behavior/ Proposed in that article fix is the same I've tried before document.addEventListener('click', event => {
if (event.target.matches('button') {
event.target.focus()
}
}) In other words - we could recomend using this hack more widely. |
I've come across an issue regarding this fix
So I added this fix to the parent div but the issue I was having was that I tried to implement this fix but no dice
|
The only "working" solution, as described above
No real solution today :( Except for the way adopted by reakit - DON'T USE "BUTTONS" - see https://twitter.com/diegohaz/status/1176998100495491072 |
This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far. |
Problem
In macOS Safari, clicking links below the fold inside the
FocusLock
component causes focus to jump to the first link instead of going the destination of the link.Here's a codesandbox that reproduces the issue: https://codesandbox.io/s/responsive-navigation-safari-bug-6hvj4
Notes
Removing the
FocusLock
component from theDrawer
component fixes this issue.Removing
tabIndex="-1"
from the wrapper div created by @reach/router also fixes this issue:Related to primer/doctocat#56
The text was updated successfully, but these errors were encountered: