Bidirectional FocusZone with checkForNoWrap not working correctly.

[cvirendra]

There's a bug in FocusZone where if active element is the left most element in any row (except top row) AND the top row is not in scroll view, pressing left arrow button moves focus to first element in the top row rather than doing nothing.

This works fine when all the elements are visible on the screen.

You can see this in action here. Click on 1st item in the 4th row and press left arrow key. Instead of doing nothing, the first 1st item in 1st row gets selected.

https://codepen.io/cvirendra/pen/dBOjZO

[cvirendra]

Passing .toFixed output to parseFloat in _moveFocusLeft (in FocusZone.tsx) method when calculating topBottomComparison fixes this issue.

[aftab-hassan]

@cvirendra thank you for reporting the issue and providing a codepen.

Passing .toFixed output to parseFloat in _moveFocusLeft (in FocusZone.tsx) method when calculating topBottomComparison fixes this issue.

@JasonGore @natalieethell, wanted to get your thoughts here. Does the suggested solution sound reasonable ?

[citelao]

I'm seeing another issue here where left and right arrow keys do not work in the top row no matter what (same CodePen). Is that related?

Happy to file another bug.

[aftab-hassan]

I'm seeing another issue here where left and right arrow keys do not work in the top row no matter what (same CodePen). Is that related?

Happy to file another bug.

thanks @citelao, I noticed the same. We'll just track it here. Wanted to get more input from the team on the behavior of checkForNoWrap before we file another issue for the same.

[natalieethell]

@cvirendra thanks for letting us know about this and proposing a fix! I do see that adding parseFloat fixes the issue, but I can't seem to figure out why... Do you have a good explanation of as to why? I'm just curious - it's awesome that it fixes it.

If we did go with this approach, we'd also want to make sure we do the same for _moveFocusRight and the RTL cases in both _moveFocusLeft and _moveFocusRight.

[natalieethell]

@citelao it looks like adding parseFloat fixes those other weird issues you're seeing, too

[cvirendra]

@natalieethell toFixed returns string. So without using parseFloat

topBottomComparison = targetRect.top.toFixed(3) < activeRect.bottom.toFixed(3);

is comparing strings and may not always get us the desired result.

You're correct about making this change everywhere toFixed is used in FocusZone.tsx. I have this change (locally) in both _moveFocusLeft and _moveFocusRight.

[aftab-hassan]

@cvirendra, thank you for the explanation and great to know you have a local enlistment working as well. The string comparison makes sense.

Would you be interested in submitting a PR for this? We can have a codeowner take a look at it.
Here are some Contributing guidelines to get you started: https://github.com/OfficeDev/office-ui-fabric-react/wiki/Contributing