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

Tabs losing focus on keyboard interaction

Open rlax opened this issue 7 years ago • 4 comments

I was importing react-tabs component into my application and using them in my components. There is a minor bug: when I am focusing on Tab with keyboard and try to move between tabs with left\right keys they will switch, correctly show new tab content, but lose focus afterwards, so you cannot press right\left to switch tab again.

After some investigation I found the cause for this: inside UncontrolledTabs component there is a check for document.activeElement availability. This check returns null for document.activeElement and thus canUseActiveElement will be set to false. I couldn't reproduce this case in a codesandbox or other environments, document.activeElement returned <body>...</body> element elsewhere.

So, on to the questions:

  1. Do I need to make a PR for fix? There is a number of other issues in this repo concerning similar situations, but all of them were closed with "can't reproduce" reason. I managed to found getActiveElement implementation in React repo, and there they return document.body if document.activeElement returned null.
  2. According to MSDN doc on activeElement it will return <body> or null, but i cant quite get a grip when null will be returned. Tried to find some clues inside whatwg spec but it was to no avail. I get that there is may be no window.document.activeElement when react app is processed but still, how exactly does this works?
  3. In addition to my 2nd question, why even make this check inside the UncontrolledTabs component outside class declaration, would it be more correct to check in lifecycle method or constructor?

Hope I made my issue clear to understand, looking forward to solve it!

rlax avatar Sep 20 '18 12:09 rlax

First off, I really appreciate the research you put into this. I think I'm missing some information here though — what browser are you facing this issue in? From the links in the code comments it seems like the issue was fixed in Edge over a year ago, and even then it was only an issue inside iframes. Are you 100% sure canUseActiveElement is set to false in your case?

As for point 3, I'm guessing the reason is that it was designed to be a feature detection mechanism, so it makes sense that it would only be executed once (the assumption being that the outcome would never change).

PS I remember having the same or similar problems with versions of react-tabs < 1.0, but they were fixed when 1.0 was released and haven't regressed as far as I can tell.

joepvl avatar Sep 21 '18 10:09 joepvl

I am facing this in FF, Chrome, didn't test it with Edge. I am aware about IE fix for activeElement, it was fixed here. But the fix was about activeElement in IE, and in my situation feature detection is working fine, but returns null from document.activeElement and double negating to false.

P.S. canUseActiveElement is set to false, document.activeElement set to null, I am pretty sure. Curious why is it different in codesandbox (returning <body>... there)

P.P.S. I ran tests on other envs and projects, and <body>.. is returned elsewhere. But in my project it is null. Could the bug appear only on react<16 or with lower webpack versions? I think this versions are irrelevant to the case.

rlax avatar Sep 21 '18 12:09 rlax

Switched my project to react16 to no avail. Excerpt from my code for information: UncontrolledTabs.js try { console.log(window.document.activeElement); canUseActiveElement = !!(typeof window !== 'undefined' && window.document && window.document.activeElement); } catch (e) {... After app build and start null is logged.

rlax avatar Sep 21 '18 15:09 rlax

~Not sure if this is a separate issue but the same thing happens to me as soon as I pass any prop to the <Tabs> component (e.g. onSelect). When I do that the componentWillReceiveProps will fire before each render and that forces the focus to be removed by calling copyPropsToState without passing a value for focus which has false as the default value.~

~If I remove onSelect everything works fine because componentWillReceiveProps is never called, but I need that function so I can do some additional UI changes.~

EDIT: It seems it was more complicated than that and has something to do with how I was wrapping the component.

baldurh avatar Dec 02 '18 18:12 baldurh