Skip to content

fixing focusZone algnment issue #4488

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.

Sign up for GitHub

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

Merged
dzearing merged 7 commits into from
Apr 11, 2018
Merged

fixing focusZone algnment issue #4488

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4da07da
fixing focusZone algnment issue
lorejoh12 Apr 7, 2018
4863440
updating snapshots
lorejoh12 Apr 9, 2018
b85c372
pushing a missed snapshot test
lorejoh12 Apr 9, 2018
4165369
Merge branch 'master' of https://github.com/officedev/office-ui-fabri…
lorejoh12 Apr 9, 2018
8103e99
pulling from master, fixing test that was recently added that was broken
lorejoh12 Apr 9, 2018
f85d088
responding to david's comments
lorejoh12 Apr 10, 2018
736ea88
Update jolore-focusZoneUpdate_2018-04-07-00-43.json
dzearing Apr 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions packages/office-ui-fabric-react/src/components/FocusZone/FocusZone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export class FocusZone extends BaseComponent<IFocusZoneProps, {}> implements IFo
while (parentElement && parentElement !== this._root.value) {
if (isElementTabbable(parentElement) && this._isImmediateDescendantOfZone(parentElement)) {
this._activeElement = parentElement;
this._setFocusAlignment(this._activeElement);

@dzearing dzearing Apr 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will cause focus to be "pulled" to one side for variable sized boxes.

@lorejoh12 lorejoh12 Apr 9, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

break;
}
parentElement = getParent(parentElement, ALLOW_VIRTUAL_ELEMENTS) as HTMLElement;
Expand Down Expand Up @@ -696,24 +697,13 @@ export class FocusZone extends BaseComponent<IFocusZoneProps, {}> implements IFo
}

private _setFocusAlignment(element: HTMLElement, isHorizontal?: boolean, isVertical?: boolean) {
if (this.props.direction === FocusZoneDirection.bidirectional &&
(!this._focusAlignment || isHorizontal || isVertical)) {
if (this.props.direction === FocusZoneDirection.bidirectional) {

const rect = element.getBoundingClientRect();
const left = rect.left + (rect.width / 2);
const top = rect.top + (rect.height / 2);

if (!this._focusAlignment) {
this._focusAlignment = { left, top };
}

if (isHorizontal) {
this._focusAlignment.left = left;
}

if (isVertical) {
this._focusAlignment.top = top;
}
this._focusAlignment = { left, top };
}
}

Expand Down