From 8f1fce1cb9e9a3dd0271b78260e8ba3010449743 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Mon, 7 May 2018 19:02:57 -0700 Subject: [PATCH 01/11] added fix for alt + down on menu for a menu button and not just split button --- .../src/components/Button/BaseButton.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 2363f38db1e71f..3b554311f5aa0b 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -450,7 +450,7 @@ export class BaseButton extends BaseComponent { if (this.props.menuProps) { - const menuProps = {...this.props.menuProps, shouldFocusOnContainer: shouldFocusOnContainer }; + const menuProps = { ...this.props.menuProps, shouldFocusOnContainer: shouldFocusOnContainer }; if (this.props.persistMenu) { menuProps.hidden = false; } @@ -679,12 +679,8 @@ export class BaseButton extends BaseComponent): boolean { if (this.props.menuTriggerKeyCode) { return ev.which === this.props.menuTriggerKeyCode; - } else { - if (this._isSplitButton) { - return ev.which === KeyCodes.down && (ev.altKey || ev.metaKey); - } else { - return ev.which === KeyCodes.enter; - } + } else if (this.props.menuProps) { + return ev.which === KeyCodes.down && (ev.altKey || ev.metaKey); } } From fda1afecbcdc27407665ce8cf2f638bccff08e17 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Mon, 7 May 2018 19:09:26 -0700 Subject: [PATCH 02/11] added change file; added comment --- .../master_2018-05-08-02-09.json | 11 +++++++++++ .../src/components/Button/BaseButton.tsx | 3 +++ 2 files changed, 14 insertions(+) create mode 100644 common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json diff --git a/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json b/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json new file mode 100644 index 00000000000000..c131957538deb1 --- /dev/null +++ b/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "BaseButton: Allow Alt + Down on menu buttons to open the menu", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "chiechan@microsoft.com" +} \ No newline at end of file diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 3b554311f5aa0b..46eeb333f4182c 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -639,6 +639,9 @@ export class BaseButton extends BaseComponent void = () => { From 5515aae61cf50bfb09129ab8d361d1c297d1f728 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Mon, 18 Jun 2018 17:41:23 -0700 Subject: [PATCH 03/11] removed unneeded change file --- .../master_2018-05-08-02-09.json | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json diff --git a/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json b/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json deleted file mode 100644 index c131957538deb1..00000000000000 --- a/common/changes/office-ui-fabric-react/master_2018-05-08-02-09.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "changes": [ - { - "packageName": "office-ui-fabric-react", - "comment": "BaseButton: Allow Alt + Down on menu buttons to open the menu", - "type": "patch" - } - ], - "packageName": "office-ui-fabric-react", - "email": "chiechan@microsoft.com" -} \ No newline at end of file From 44b7f2a0f10cf5447253e653aed4d30da1b90687 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Tue, 14 Aug 2018 18:46:50 -0700 Subject: [PATCH 04/11] removed unnecessary call to put focus on the base button --- .../src/components/Button/BaseButton.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 1009e65c219d36..4d406b76a1eed1 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -446,10 +446,6 @@ export class BaseButton extends BaseComponent { - if (this._splitButtonContainer.current) { - this._splitButtonContainer.current.focus(); - } - const currentMenuProps = this.state.menuProps; if (this.props.persistMenu) { currentMenuProps && currentMenuProps.hidden ? this._openMenu(shouldFocusOnContainer) : this._dismissMenu(); From 348cf942fa6dd1f555781b79597c3bfba3701912 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Tue, 14 Aug 2018 19:04:01 -0700 Subject: [PATCH 05/11] add change files --- ...an-stopAgressiveFocusInMenus_2018-08-15-02-03.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json diff --git a/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json new file mode 100644 index 00000000000000..ef23b54b51eb47 --- /dev/null +++ b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "BaseButton: Remove us unncessary putting focus on the split button we toggle a menu", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "chiechan@microsoft.com" +} \ No newline at end of file From d088b28f8a9738cb8a8d7ef6d4922cb6905c5276 Mon Sep 17 00:00:00 2001 From: Josh Chang Date: Wed, 15 Aug 2018 11:19:52 -0700 Subject: [PATCH 06/11] Update chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json --- .../chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json index ef23b54b51eb47..d9bb9ad392e6ce 100644 --- a/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json +++ b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json @@ -2,10 +2,10 @@ "changes": [ { "packageName": "office-ui-fabric-react", - "comment": "BaseButton: Remove us unncessary putting focus on the split button we toggle a menu", + "comment": "BaseButton: Remove unnecessary putting focus on the split button we toggle a menu", "type": "patch" } ], "packageName": "office-ui-fabric-react", "email": "chiechan@microsoft.com" -} \ No newline at end of file +} From a45247c236aaec69e78a5062cf1f6e972aae2c7f Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Thu, 16 Aug 2018 11:51:54 -0700 Subject: [PATCH 07/11] revert previous change with focusing the split button container; contextual menu: added onMenuDismissed to be after we focus the previous active element --- .../src/components/Button/BaseButton.tsx | 3 +++ .../ContextualMenu/ContextualMenu.base.tsx | 15 ++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index e0b68417d1af98..46c14566d5c998 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -454,6 +454,9 @@ export class BaseButton extends BaseComponent { const currentMenuProps = this.state.menuProps; + if (this._splitButtonContainer.current) { + this._splitButtonContainer.current.focus(); + } let shouldFocusOnMount = true; if (this.props.menuProps && this.props.menuProps.shouldFocusOnMount === false) { shouldFocusOnMount = false; diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx index 40576ef788cdc0..95ccb863310df2 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx @@ -163,13 +163,9 @@ export class ContextualMenuBase extends BaseComponent { - this._previousActiveElement && this._previousActiveElement!.focus(); - }, 0); - } - - if (this.props.onMenuDismissed) { - this.props.onMenuDismissed(this.props); + setTimeout(() => this._focusPreviousElement(), 0); + } else { + this.props.onMenuDismissed && this.props.onMenuDismissed(this.props); } this._events.dispose(); @@ -1103,4 +1099,9 @@ export class ContextualMenuBase extends BaseComponent | PointerEvent) => { this._cancelSubMenuTimer(); }; + + private _focusPreviousElement = (): void => { + this._previousActiveElement && this._previousActiveElement!.focus(); + this.props.onMenuDismissed && this.props.onMenuDismissed(this.props); + }; } From e1902abd0d4e01ad2e316b953e44ec2fab333edd Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Thu, 16 Aug 2018 11:54:54 -0700 Subject: [PATCH 08/11] removed unnecessary base button change --- .../src/components/Button/BaseButton.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 46c14566d5c998..00ce6d9c4cfba3 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -453,10 +453,11 @@ export class BaseButton extends BaseComponent { - const currentMenuProps = this.state.menuProps; if (this._splitButtonContainer.current) { this._splitButtonContainer.current.focus(); } + + const currentMenuProps = this.state.menuProps; let shouldFocusOnMount = true; if (this.props.menuProps && this.props.menuProps.shouldFocusOnMount === false) { shouldFocusOnMount = false; From 4b894b70185d519b5ef54f81fe908b32ac00f934 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Thu, 16 Aug 2018 18:10:51 -0700 Subject: [PATCH 09/11] add split button container onfocus capture event to keep focus on the split button container --- .../src/components/Button/BaseButton.tsx | 13 +++++++++---- .../src/components/Button/Button.types.ts | 5 +++++ .../ContextualMenu/ContextualMenu.base.tsx | 15 +++++++-------- .../Button.CommandBar.Example.tsx.shot | 1 + .../__snapshots__/Button.Split.Example.tsx.shot | 5 +++++ .../FocusZone.Tabbable.Example.tsx.shot | 1 + .../__snapshots__/Keytips.Button.Example.tsx.shot | 1 + 7 files changed, 29 insertions(+), 12 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 00ce6d9c4cfba3..3f1152dfc1a92b 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -453,10 +453,6 @@ export class BaseButton extends BaseComponent { - if (this._splitButtonContainer.current) { - this._splitButtonContainer.current.focus(); - } - const currentMenuProps = this.state.menuProps; let shouldFocusOnMount = true; if (this.props.menuProps && this.props.menuProps.shouldFocusOnMount === false) { @@ -523,6 +519,7 @@ export class BaseButton extends BaseComponent {this._onRenderContent(tag, buttonProps)} @@ -535,6 +532,14 @@ export class BaseButton extends BaseComponent) => { + if (this.props.onSplitContainerFocusCapture) { + this.props.onSplitContainerFocusCapture(ev); + } + // stops the event from propagating so focus remains on the SplitButton Container + ev.stopPropagation(); + }; + private _onSplitButtonPrimaryClick = (ev: React.MouseEvent) => { if (this._isExpanded) { this._dismissMenu(); diff --git a/packages/office-ui-fabric-react/src/components/Button/Button.types.ts b/packages/office-ui-fabric-react/src/components/Button/Button.types.ts index 3d44964f813269..1c5991943f42be 100644 --- a/packages/office-ui-fabric-react/src/components/Button/Button.types.ts +++ b/packages/office-ui-fabric-react/src/components/Button/Button.types.ts @@ -283,6 +283,11 @@ export interface IButtonProps * @deprecated Use 'secondaryText' instead. */ description?: IStyle; + + /** + * Callback for when the split button container has a onFocus capture event + */ + onSplitContainerFocusCapture?: (ev?: React.FocusEvent) => void; } export enum ElementType { diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx index 95ccb863310df2..40576ef788cdc0 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx @@ -163,9 +163,13 @@ export class ContextualMenuBase extends BaseComponent this._focusPreviousElement(), 0); - } else { - this.props.onMenuDismissed && this.props.onMenuDismissed(this.props); + setTimeout(() => { + this._previousActiveElement && this._previousActiveElement!.focus(); + }, 0); + } + + if (this.props.onMenuDismissed) { + this.props.onMenuDismissed(this.props); } this._events.dispose(); @@ -1099,9 +1103,4 @@ export class ContextualMenuBase extends BaseComponent | PointerEvent) => { this._cancelSubMenuTimer(); }; - - private _focusPreviousElement = (): void => { - this._previousActiveElement && this._previousActiveElement!.focus(); - this.props.onMenuDismissed && this.props.onMenuDismissed(this.props); - }; } diff --git a/packages/office-ui-fabric-react/src/components/__snapshots__/Button.CommandBar.Example.tsx.shot b/packages/office-ui-fabric-react/src/components/__snapshots__/Button.CommandBar.Example.tsx.shot index 07d07716619ea4..7b8cbf6891881b 100644 --- a/packages/office-ui-fabric-react/src/components/__snapshots__/Button.CommandBar.Example.tsx.shot +++ b/packages/office-ui-fabric-react/src/components/__snapshots__/Button.CommandBar.Example.tsx.shot @@ -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" diff --git a/packages/office-ui-fabric-react/src/components/__snapshots__/Button.Split.Example.tsx.shot b/packages/office-ui-fabric-react/src/components/__snapshots__/Button.Split.Example.tsx.shot index 339397489c928a..2aefff8d3eac93 100644 --- a/packages/office-ui-fabric-react/src/components/__snapshots__/Button.Split.Example.tsx.shot +++ b/packages/office-ui-fabric-react/src/components/__snapshots__/Button.Split.Example.tsx.shot @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" diff --git a/packages/office-ui-fabric-react/src/components/__snapshots__/FocusZone.Tabbable.Example.tsx.shot b/packages/office-ui-fabric-react/src/components/__snapshots__/FocusZone.Tabbable.Example.tsx.shot index 606538cfc0caff..d614d8e2196fa1 100644 --- a/packages/office-ui-fabric-react/src/components/__snapshots__/FocusZone.Tabbable.Example.tsx.shot +++ b/packages/office-ui-fabric-react/src/components/__snapshots__/FocusZone.Tabbable.Example.tsx.shot @@ -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" diff --git a/packages/office-ui-fabric-react/src/components/__snapshots__/Keytips.Button.Example.tsx.shot b/packages/office-ui-fabric-react/src/components/__snapshots__/Keytips.Button.Example.tsx.shot index ba3cccb2928f7d..f1fea458aee348 100644 --- a/packages/office-ui-fabric-react/src/components/__snapshots__/Keytips.Button.Example.tsx.shot +++ b/packages/office-ui-fabric-react/src/components/__snapshots__/Keytips.Button.Example.tsx.shot @@ -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" From bb3aa7184c60d5846d031aa048bf04c7221a7f40 Mon Sep 17 00:00:00 2001 From: "REDMOND\\chiechan" Date: Fri, 17 Aug 2018 13:45:11 -0700 Subject: [PATCH 10/11] get rid of the splitContainerfocus callback; and moved togglemenu split button container focus to the focus capture in the split container --- .../src/components/Button/BaseButton.tsx | 8 +++----- .../src/components/Button/Button.types.ts | 5 ----- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx index 3f1152dfc1a92b..dd4685fed1b403 100644 --- a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx +++ b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx @@ -533,11 +533,9 @@ export class BaseButton extends BaseComponent) => { - if (this.props.onSplitContainerFocusCapture) { - this.props.onSplitContainerFocusCapture(ev); - } - // stops the event from propagating so focus remains on the SplitButton Container - ev.stopPropagation(); + // 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) => { diff --git a/packages/office-ui-fabric-react/src/components/Button/Button.types.ts b/packages/office-ui-fabric-react/src/components/Button/Button.types.ts index 1c5991943f42be..3d44964f813269 100644 --- a/packages/office-ui-fabric-react/src/components/Button/Button.types.ts +++ b/packages/office-ui-fabric-react/src/components/Button/Button.types.ts @@ -283,11 +283,6 @@ export interface IButtonProps * @deprecated Use 'secondaryText' instead. */ description?: IStyle; - - /** - * Callback for when the split button container has a onFocus capture event - */ - onSplitContainerFocusCapture?: (ev?: React.FocusEvent) => void; } export enum ElementType { From 35c1e2bf16636e48272a6e331d319f81a251c776 Mon Sep 17 00:00:00 2001 From: Josh Chang Date: Fri, 17 Aug 2018 14:27:40 -0700 Subject: [PATCH 11/11] Updated change list --- .../chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json index d9bb9ad392e6ce..aeb2df3bfa8706 100644 --- a/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json +++ b/common/changes/office-ui-fabric-react/chiechan-stopAgressiveFocusInMenus_2018-08-15-02-03.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "office-ui-fabric-react", - "comment": "BaseButton: Remove unnecessary putting focus on the split button we toggle a menu", + "comment": "BaseButton: Add onFocusCapture to the split button container to focus the container instead of doing it in the menu onClick", "type": "patch" } ],