Button: Conditionally rendering KeytipData (#9276)

* Adding conditional rendering of KeytipData in oufr Button. * Moving Tag inside KeytipData and adding the same conditional rendering to oufr split buttons. * Calling function instead of using JSX Element for Keytips. * Calling function instead of using JSX Element for Keytips in Button.view
microsoft · Jun 3, 2019 · 5da0db9 · 5da0db9
1 parent 1226e08
commit 5da0db9
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 65 deletions.
diff --git a/common/changes/@uifabric/experiments/buttonKeytip_2019-05-29-23-51.json b/common/changes/@uifabric/experiments/buttonKeytip_2019-05-29-23-51.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/experiments",
+      "comment": "Button: Conditionally rendering KeytipData.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@uifabric/experiments",
+  "email": "Humberto.Morimoto@microsoft.com"
+}
diff --git a/common/changes/office-ui-fabric-react/buttonKeytip_2019-05-29-23-51.json b/common/changes/office-ui-fabric-react/buttonKeytip_2019-05-29-23-51.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Button: Conditionally rendering KeytipData.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "Humberto.Morimoto@microsoft.com"
+}
diff --git a/packages/experiments/src/components/Button/Button.view.tsx b/packages/experiments/src/components/Button/Button.view.tsx
@@ -29,29 +29,33 @@ export const ButtonView: IButtonComponent['view'] = props => {
     }
   };
 
-  return (
+  const Button = (keytipAttributes?: any): JSX.Element => (
+    <Slots.root
+      type="button" // stack doesn't take in native button props
+      role="button"
+      onClick={_onClick}
+      {...buttonProps}
+      {...keytipAttributes}
+      disabled={disabled && !allowDisabledFocus}
+      aria-disabled={disabled}
+      tabIndex={!disabled || allowDisabledFocus ? 0 : undefined}
+      aria-label={ariaLabel}
+      ref={buttonRef}
+    >
+      <Slots.stack horizontal as="span" tokens={{ childrenGap: 8 }} verticalAlign="center" horizontalAlign="center" verticalFill>
+        <Slots.icon />
+        <Slots.content />
+        {children}
+      </Slots.stack>
+    </Slots.root>
+  );
+
+  return keytipProps ? (
     <KeytipData keytipProps={keytipProps} disabled={disabled && !allowDisabledFocus}>
-      {(keytipAttributes: any): JSX.Element => (
-        <Slots.root
-          type="button" // stack doesn't take in native button props
-          role="button"
-          onClick={_onClick}
-          {...buttonProps}
-          {...keytipAttributes}
-          disabled={disabled && !allowDisabledFocus}
-          aria-disabled={disabled}
-          tabIndex={!disabled || allowDisabledFocus ? 0 : undefined}
-          aria-label={ariaLabel}
-          ref={buttonRef}
-        >
-          <Slots.stack horizontal as="span" tokens={{ childrenGap: 8 }} verticalAlign="center" horizontalAlign="center" verticalFill>
-            <Slots.icon />
-            <Slots.content />
-            {children}
-          </Slots.stack>
-        </Slots.root>
-      )}
+      {(keytipAttributes: any): JSX.Element => Button(keytipAttributes)}
     </KeytipData>
+  ) : (
+    Button()
   );
 };
 

diff --git a/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx b/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx
@@ -274,28 +274,32 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
       };
     }
 
-    const Content = (
+    const Button = (keytipAttributes?: any): JSX.Element => (
+      <Tag {...buttonProps} {...keytipAttributes}>
+        <div className={this._classNames.flexContainer}>
+          {onRenderIcon(props, this._onRenderIcon)}
+          {this._onRenderTextContents()}
+          {onRenderAriaDescription(props, this._onRenderAriaDescription)}
+          {onRenderChildren(props, this._onRenderChildren)}
+          {!this._isSplitButton &&
+            (menuProps || menuIconProps || this.props.onRenderMenuIcon) &&
+            onRenderMenuIcon(this.props, this._onRenderMenuIcon)}
+          {this.state.menuProps && !this.state.menuProps.doNotLayer && onRenderMenu(menuProps, this._onRenderMenu)}
+        </div>
+      </Tag>
+    );
+
+    const Content = keytipProps ? (
       // If we're making a split button, we won't put the keytip here
       <KeytipData
         keytipProps={!this._isSplitButton ? keytipProps : undefined}
         ariaDescribedBy={(buttonProps as any)['aria-describedby']}
         disabled={disabled}
       >
-        {(keytipAttributes: any): JSX.Element => (
-          <Tag {...buttonProps} {...keytipAttributes}>
-            <div className={this._classNames.flexContainer}>
-              {onRenderIcon(props, this._onRenderIcon)}
-              {this._onRenderTextContents()}
-              {onRenderAriaDescription(props, this._onRenderAriaDescription)}
-              {onRenderChildren(props, this._onRenderChildren)}
-              {!this._isSplitButton &&
-                (menuProps || menuIconProps || this.props.onRenderMenuIcon) &&
-                onRenderMenuIcon(this.props, this._onRenderMenuIcon)}
-              {this.state.menuProps && !this.state.menuProps.doNotLayer && onRenderMenu(menuProps, this._onRenderMenu)}
-            </div>
-          </Tag>
-        )}
+        {(keytipAttributes: any): JSX.Element => Button(keytipAttributes)}
       </KeytipData>
+    ) : (
+      Button()
     );
 
     if (menuProps && menuProps.doNotLayer) {
@@ -492,36 +496,41 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
     }
 
     const containerProps = getNativeProps(buttonProps, [], ['disabled']);
-    return (
+
+    const SplitButton = (keytipAttributes?: any): JSX.Element => (
+      <div
+        {...containerProps}
+        data-ktp-target={keytipAttributes ? keytipAttributes['data-ktp-target'] : undefined}
+        role={'button'}
+        aria-disabled={disabled}
+        aria-haspopup={true}
+        aria-expanded={this._isExpanded}
+        aria-pressed={toggle ? !!checked : undefined} // aria-pressed attribute should only be present for toggle buttons
+        aria-describedby={mergeAriaAttributeValues(ariaDescribedBy, keytipAttributes ? keytipAttributes['aria-describedby'] : undefined)}
+        className={classNames && classNames.splitButtonContainer}
+        onKeyDown={this._onSplitButtonContainerKeyDown}
+        onTouchStart={this._onTouchStart}
+        ref={this._splitButtonContainer}
+        data-is-focusable={true}
+        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)}
+          {this._onRenderSplitButtonMenuButton(classNames, keytipAttributes)}
+          {this._onRenderSplitButtonDivider(classNames)}
+        </span>
+      </div>
+    );
+
+    return keytipProps ? (
       <KeytipData keytipProps={keytipProps} disabled={disabled}>
-        {(keytipAttributes: any): JSX.Element => (
-          <div
-            {...containerProps}
-            data-ktp-target={keytipAttributes['data-ktp-target']}
-            role={'button'}
-            aria-disabled={disabled}
-            aria-haspopup={true}
-            aria-expanded={this._isExpanded}
-            aria-pressed={toggle ? !!checked : undefined} // aria-pressed attribute should only be present for toggle buttons
-            aria-describedby={mergeAriaAttributeValues(ariaDescribedBy, keytipAttributes['aria-describedby'])}
-            className={classNames && classNames.splitButtonContainer}
-            onKeyDown={this._onSplitButtonContainerKeyDown}
-            onTouchStart={this._onTouchStart}
-            ref={this._splitButtonContainer}
-            data-is-focusable={true}
-            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)}
-              {this._onRenderSplitButtonMenuButton(classNames, keytipAttributes)}
-              {this._onRenderSplitButtonDivider(classNames)}
-            </span>
-          </div>
-        )}
+        {(keytipAttributes: any): JSX.Element => SplitButton(keytipAttributes)}
       </KeytipData>
+    ) : (
+      SplitButton()
     );
   }
 
@@ -587,7 +596,7 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
     return (
       <BaseButton
         {...splitButtonProps}
-        data-ktp-execute-target={keytipAttributes['data-ktp-execute-target']}
+        data-ktp-execute-target={keytipAttributes ? keytipAttributes['data-ktp-execute-target'] : keytipAttributes}
         onMouseDown={this._onMouseDown}
         tabIndex={-1}
       />