fixing focusZone algnment issue#4488
Conversation
|
To see the issue, go to the test page, click on the item well with 2 lines of items, and try arrowing around. If you arrow to the bottom of the well, then start arrowing back to the top, it doesn't update the focus alignment, so you go back to where you came from. Odd but potentially reasonable. But now if you click on an item, the focus alignment still doesn't update. So if your focus is on the first column, you click in the middle, and then try to arrow downwards, the focus shifts back to the first column. This seems like a bug.
|
| while (parentElement && parentElement !== this._root.value) { | ||
| if (isElementTabbable(parentElement) && this._isImmediateDescendantOfZone(parentElement)) { | ||
| this._activeElement = parentElement; | ||
| this._setFocusAlignment(this._activeElement); |
There was a problem hiding this comment.
I think this will cause focus to be "pulled" to one side for variable sized boxes.
There was a problem hiding this comment.
I don't think so. The code at lines 216-217 run when the event target is the immediate descendent of the parent. If it's not, this while loop just finds the parent element that is an immediate descendent, and should therefore be running the same code once it finds that parent element. Looks like a bug that we were only updating focus alignment if the focused element was a top level element.
The case where focus gets pulled to the side was caused by my changes down in _setFocusAlignment(), e.g. we only want to update the horizontal alignment when going left or right, the onFocus shouldn't update the alignment except for the case where no alignment was previously set.
lorejoh12
requested review from
cschleiden,
joschect and
micahgodbolt
as code owners
| "changes": [ | ||
| { | ||
| "comment": "", | ||
| "packageName": "@uifabric/example-app-base", |
There was a problem hiding this comment.
can you delete all these "type": "none" files?
I believe this is because your "upstream" remote url needs to be exactly
https://github.com/OfficeDev/office-ui-fabric-react.git
| onFocus={ this._onFocus } | ||
| onClick={ this._onClick } | ||
| onMouseDownCapture={ this._onMouseDown } | ||
| > |
There was a problem hiding this comment.
can you add a test case to capture this?
There was a problem hiding this comment.
added to FocusZone.test.tsx
dzearing
left a comment
dzearing
left a comment
There was a problem hiding this comment.
minor feedback! Thank you for fixing.
2 participants
Pull request checklist
$ npm run changeDescription of changes
Focus zone was not updating alignment on focus coming in. So clicking, and then trying to arrow, was not keeping track of where it was supposed to go. In addition, arrowing all the way to the bottom of the focus zone goes to the bottom row, then over to the bottom right corner, but an up arrow (which should go up to the one above) goes back up to a random element in the middle. These were caused by the focus alignment not updating.
Focus areas to test
(optional)