Skip to content

"BaseButton: Add onFocusCapture to the split button container to focus the container instead of doing it in the menu onClick #5934

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
chang47 merged 17 commits into from
Aug 20, 2018
Merged

"BaseButton: Add onFocusCapture to the split button container to focus the container instead of doing it in the menu onClick #5934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8f1fce1
added fix for alt + down on menu for a menu button and not just split…
May 8, 2018
fda1afe
added change file; added comment
May 8, 2018
20f616d
Merge https://github.com/OfficeDev/office-ui-fabric-react
May 10, 2018
1dd174c
Merge https://github.com/OfficeDev/office-ui-fabric-react
May 14, 2018
dcdad51
resolved merge
Jun 13, 2018
5515aae
removed unneeded change file
Jun 19, 2018
817103b
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
Jun 21, 2018
44b7f2a
removed unnecessary call to put focus on the base button
Aug 15, 2018
348cf94
add change files
Aug 15, 2018
d088b28
Update chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json
chang47 Aug 15, 2018
a1aa639
Merge https://github.com/OfficeDev/office-ui-fabric-react into chiech…
Aug 15, 2018
959eaa1
Merge branch 'chiechan/stopAgressiveFocusInMenus' of https://github.c…
Aug 15, 2018
a45247c
revert previous change with focusing the split button container; cont…
Aug 16, 2018
e1902ab
removed unnecessary base button change
Aug 16, 2018
4b894b7
add split button container onfocus capture event to keep focus on the…
Aug 17, 2018
bb3aa71
get rid of the splitContainerfocus callback; and moved togglemenu spl…
Aug 17, 2018
35c1e2b
Updated change list
chang47 Aug 17, 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
11 changes: 11 additions & 0 deletions ...n/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "BaseButton: Add onFocusCapture to the split button container to focus the container instead of doing it in the menu onClick",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
11 changes: 7 additions & 4 deletions packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,6 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
};

private _onToggleMenu = (shouldFocusOnContainer: boolean): void => {
if (this._splitButtonContainer.current) {
this._splitButtonContainer.current.focus();
}
Copy link
Contributor

@jspurlin jspurlin Aug 15, 2018

Choose a reason for hiding this comment

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

Is this the right fix? It looks like you added it in March to fix some specific issues: #4222

Copy link
Contributor Author

@chang47 chang47 Aug 17, 2018

Choose a reason for hiding this comment

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

Like we talked offline, we're getting rid of this and instead we're using the onFocus capture event to ensure that focus is set to the SplitButton Container instead of us manually putting it there.


const currentMenuProps = this.state.menuProps;
let shouldFocusOnMount = true;
if (this.props.menuProps && this.props.menuProps.shouldFocusOnMount === false) {
Expand Down Expand Up @@ -523,6 +519,7 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
onClick={!disabled && !primaryDisabled ? this._onSplitButtonPrimaryClick : undefined}
tabIndex={!disabled || allowDisabledFocus ? 0 : undefined}
aria-roledescription={buttonProps['aria-roledescription']}
onFocusCapture={this._onSplitContainerFocusCapture}
>
<span style={{ display: 'flex' }}>
{this._onRenderContent(tag, buttonProps)}
Expand All @@ -535,6 +532,12 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
);
}

private _onSplitContainerFocusCapture = (ev: React.FocusEvent<HTMLDivElement>) => {
// We should never be able to focus the individual buttons in a split button. Focus
// should always remain on the container.
this._splitButtonContainer.current && this._splitButtonContainer.current.focus();
};

private _onSplitButtonPrimaryClick = (ev: React.MouseEvent<HTMLDivElement>) => {
if (this._isExpanded) {
this._dismissMenu();
Expand Down
1 change: 1 addition & 0 deletions ...es/office-ui-fabric-react/src/components/__snapshots__/Button.CommandBar.Example.tsx.shot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ exports[`Component Examples renders Button.CommandBar.Example.tsx correctly 1`]
data-is-focusable={true}
data-ktp-target={undefined}
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down
5 changes: 5 additions & 0 deletions packages/office-ui-fabric-react/src/components/__snapshots__/Button.Split.Example.tsx.shot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ exports[`Component Examples renders Button.Split.Example.tsx correctly 1`] = `
data-is-focusable={true}
data-ktp-target={undefined}
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down Expand Up @@ -387,6 +388,7 @@ exports[`Component Examples renders Button.Split.Example.tsx correctly 1`] = `
data-is-focusable={true}
data-ktp-target={undefined}
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down Expand Up @@ -700,6 +702,7 @@ exports[`Component Examples renders Button.Split.Example.tsx correctly 1`] = `
data-is-focusable={true}
data-ktp-target={undefined}
onClick={undefined}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down Expand Up @@ -1004,6 +1007,7 @@ exports[`Component Examples renders Button.Split.Example.tsx correctly 1`] = `
data-is-focusable={true}
data-ktp-target={undefined}
onClick={undefined}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down Expand Up @@ -1317,6 +1321,7 @@ exports[`Component Examples renders Button.Split.Example.tsx correctly 2`] = `
data-is-focusable={true}
data-ktp-target={undefined}
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down
1 change: 1 addition & 0 deletions ...s/office-ui-fabric-react/src/components/__snapshots__/FocusZone.Tabbable.Example.tsx.shot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ exports[`Component Examples renders FocusZone.Tabbable.Example.tsx correctly 1`]
data-is-focusable={true}
data-ktp-target={undefined}
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down
1 change: 1 addition & 0 deletions packages/office-ui-fabric-react/src/components/__snapshots__/Keytips.Button.Example.tsx.shot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ exports[`Component Examples renders Keytips.Button.Example.tsx correctly 1`] = `
data-is-focusable={true}
data-ktp-target="ktp-2-b"
onClick={[Function]}
onFocusCapture={[Function]}
onKeyDown={[Function]}
onTouchStart={[Function]}
role="button"
Expand Down