[Utilities] getPreviousElement does not pass "tabbable" param to all places

[sophieH29]

Reproduction of the bug in a codepen:

https://codepen.io/anon/pen/ePNjmo

Bug Report

• Package version(s): 6.75.0
• Browser and OS versions: Chrome, Mac OS

Priorities and help requested:

Are you willing to submit a PR to fix? Yes

Requested priority: Normal

Describe the issue:

Used FocusTrapZone, few elements inside have tabindex="0", others tabindex="-1", also data-is-focusable="true". I used data-is-focusable="true", as I have FocusZone inside FocusZoneTrap
When tabbing through Focus Trap Zone, lastTabbableElement is defined incorrectly what leads to tab out from trap.
Please, look at the bug reproduction here https://codepen.io/anon/pen/ePNjmo

Actual behavior:

It is possible to tab out from focus trap.

Expected behavior:

It is not possible to tab out from focus trap.

Fix

After investigating code, found out that getPreviousElement does not pass tabbable to all places, see https://github.com/OfficeDev/office-ui-fabric-react/blob/1b9405b3b64839975e16ee1fad04d7868d9e3b99/packages/utilities/src/focus.ts#L205

After trying to pass tabbable to that place, it fixed my issue.

[sophieH29]

Found a related issue #2437 and a corresponding PR #4172
It was merged, but seems it was reverted after some time.

[Vitalius1]

@sophieH29 thanks for investigating! That helps a lot.

[Vitalius1]

@joschect do you have any context on this issue?

[SpencerLynn]

Hi! I tried to fix this issue with PR #4172.

To summarize what @joschect said when my PR was being reverted:
This PR changed the default behavior with how non-tabbable elements are treated in regard to focus. This change made it so that the default behavior is that everything can be focused, which fixes the issue mentioned here. However, this breaks most other cases.

Seeing this bug has renewed my interest in looking for a fix. I'll try to take a look sometime soon.

[SpencerLynn]

I have given another attempt at fixing this issue. Hopefully this one will be more successful than my last :)

Also, he is another codepen to reproduce the issue I was attempting to fix. https://codepen.io/spencerlynn/pen/EeRdVW

[SpencerLynn]

Sorry for the spam :) My fix will not fix your scenario. However, I think if a fix for your scenario was available, it would also fix mine. You basically have simulated a FocusZone behavior with your row of images.

I'll keep working on it.

[sophieH29]

@SpencerLynn Thank you very much! I really hope we'll get it fixed.

[Jahnp]

Hey @SpencerLynn, out of curiosity, have you had any luck in your investigation?

[SpencerLynn]

@Jahnp I didn't have much luck on my investigation a while back. As the bug mentions (and where my previous fix was), the issue is with how getPreviousElement works. I haven't completely given up, but have been distracted by other things lately. So I don't have any update.