Button: Removing Stack usage to improve perf (microsoft#9516)

* Button: Removing Stack usage from slots and view to improve perf. * Adding change files for packages that were changed because of prettier. * Deleting perf log files. * Undoing prettier changes. * Removing unused change files. * Removing BaseButton changes from Actionable PR.
gingeroun · Jul 5, 2019 · 5ba5b30 · 5ba5b30
1 parent 2f42db7
commit 5ba5b30
Show file tree

Hide file tree

Showing 25 changed files with 1,199 additions and 1,933 deletions.
diff --git a/apps/vr-tests/src/stories/Card.stories.tsx b/apps/vr-tests/src/stories/Card.stories.tsx
@@ -87,16 +87,14 @@ const actionButtonStyles: IButtonStyles = {
     height: 'auto',
     minHeight: 0,
     minWidth: 0,
+    padding: 0,
 
     selectors: {
       ':hover': {
         color: '#0078D4'
       }
     }
   },
-  stack: {
-    padding: 0
-  },
   content: {
     fontSize: 12,
     fontWeight: FontWeights.semibold

diff --git a/common/changes/@uifabric/experiments/buttonStackUsage_2019-06-03-20-37.json b/common/changes/@uifabric/experiments/buttonStackUsage_2019-06-03-20-37.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/experiments",
+      "comment": " Button: Removing Stack usage from vanilla Button's slots and view to improve perf.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@uifabric/experiments",
+  "email": "[email protected]"
+}
diff --git a/common/changes/@uifabric/react-cards/buttonStackUsage_2019-06-03-20-37.json b/common/changes/@uifabric/react-cards/buttonStackUsage_2019-06-03-20-37.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/react-cards",
+      "comment": "Updating snapshots after Button changes.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@uifabric/react-cards",
+  "email": "[email protected]"
+}
diff --git a/common/changes/office-ui-fabric-react/buttonStackUsage_2019-06-24-22-13.json b/common/changes/office-ui-fabric-react/buttonStackUsage_2019-06-24-22-13.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Prettier changes.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "[email protected]"
+}
diff --git a/packages/experiments/src/components/Button/ActionButton.tsx b/packages/experiments/src/components/Button/ActionButton.tsx
@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { Button } from './Button';
-import { IButtonComponent, IButtonProps, IButtonTokenReturnType } from './Button.types';
+import { IButtonComponent, IButtonStylesReturnType, IButtonTokenReturnType } from './Button.types';
 import { ButtonVariantsType } from './ButtonVariants.types';
 import { FontWeights } from '../../Styling';
 
@@ -44,12 +44,16 @@ export const ActionButtonTokens: IButtonComponent['tokens'] = (props, theme): IB
   props.disabled && disabledTokens
 ];
 
-const ActionButtonStackProps: IButtonProps['stack'] = {
-  horizontalAlign: 'start'
+export const ActionButtonStyles: IButtonComponent['styles'] = (props, theme, tokens): IButtonStylesReturnType => {
+  return {
+    root: {
+      justifyContent: 'flex-start'
+    }
+  };
 };
 
 export const ActionButton: ButtonVariantsType = props => {
   const { text, iconProps, ...rest } = props;
 
-  return <Button stack={ActionButtonStackProps} content={text} icon={iconProps} tokens={ActionButtonTokens} {...rest} />;
+  return <Button content={text} icon={iconProps} styles={ActionButtonStyles} tokens={ActionButtonTokens} {...rest} />;
 };
diff --git a/packages/experiments/src/components/Button/Button.styles.ts b/packages/experiments/src/components/Button/Button.styles.ts
@@ -1,6 +1,7 @@
 import { IButtonComponent, IButtonStylesReturnType, IButtonTokenReturnType } from './Button.types';
 import { getFocusStyle, getGlobalClassNames, FontWeights, HighContrastSelector } from '../../Styling';
 import { IsFocusVisibleClassName } from '../../Utilities';
+import { parseGap } from 'office-ui-fabric-react/lib/components/Stack/StackUtils';
 
 const baseTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenReturnType => {
   const { effects } = theme;
@@ -10,6 +11,7 @@ const baseTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenRetur
     borderStyle: 'solid',
     borderStyleFocused: 'solid',
     borderWidth: 1,
+    childrenGap: 8,
     cursor: 'pointer',
     minWidth: 100,
     minHeight: 32,
@@ -207,6 +209,8 @@ const GlobalClassNames = {
 export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens): IButtonStylesReturnType => {
   const { className, circular } = props;
 
+  const { rowGap, columnGap } = parseGap(tokens.childrenGap, theme);
+
   const globalClassNames = getGlobalClassNames(GlobalClassNames, theme);
 
   return {
@@ -222,6 +226,7 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
       },
       theme.fonts.medium,
       {
+        alignItems: 'center',
         backgroundColor: tokens.backgroundColor,
         borderColor: tokens.borderColor,
         borderRadius: tokens.borderRadius,
@@ -230,7 +235,8 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
         boxSizing: 'border-box',
         color: tokens.color,
         cursor: tokens.cursor,
-        display: 'inline-block',
+        display: 'inline-flex',
+        flexDirection: 'row',
         fontSize: tokens.textSize,
         fontWeight: tokens.textWeight,
         height: tokens.height,
@@ -239,7 +245,7 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
         minWidth: tokens.minWidth,
         minHeight: tokens.minHeight,
         overflow: 'hidden',
-        padding: 0,
+        padding: tokens.contentPadding,
         textDecoration: 'none',
         textAlign: 'center',
         userSelect: 'none',
@@ -248,6 +254,13 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
         outlineColor: tokens.outlineColor,
 
         selectors: {
+          '> *': {
+            marginLeft: `${0.5 * rowGap.value}${rowGap.unit} ${0.5 * columnGap.value}${columnGap.unit}`,
+            textOverflow: 'ellipsis'
+          },
+          '> *:not(:first-child)': {
+            marginLeft: `${columnGap.value}${columnGap.unit}`
+          },
           [HighContrastSelector]: {
             backgroundColor: tokens.highContrastBackgroundColor,
             borderColor: tokens.highContrastBorderColor,
@@ -314,10 +327,6 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
       },
       className
     ],
-    stack: {
-      padding: tokens.contentPadding,
-      height: '100%'
-    },
     icon: [
       globalClassNames.msButtonIcon,
       {

diff --git a/packages/experiments/src/components/Button/Button.types.tsx b/packages/experiments/src/components/Button/Button.types.tsx
@@ -1,8 +1,8 @@
-import { IComponent, IComponentStyles, IHTMLElementSlot, ISlotProp, ISlottableProps, IStyleableComponentProps } from '../../Foundation';
-import { IFontWeight, IKeytipProps, IStackSlot, ITextSlot } from 'office-ui-fabric-react';
-import { IIconSlot } from '../../utilities/factoryComponents.types';
-import { IBaseProps, Omit } from '../../Utilities';
 import { IRawStyleBase } from '@uifabric/merge-styles/lib/IRawStyleBase';
+import { IFontWeight, IKeytipProps, ITextSlot } from 'office-ui-fabric-react';
+import { IComponent, IComponentStyles, IHTMLElementSlot, ISlottableProps, ISlotProp, IStyleableComponentProps } from '../../Foundation';
+import { IBaseProps, Omit } from '../../Utilities';
+import { IIconSlot } from '../../utilities/factoryComponents.types';
 
 /**
  * {@docCategory Button}
@@ -40,11 +40,6 @@ export interface IButtonSlots {
    */
   root?: IHTMLElementSlot<IButtonRootElements>;
 
-  /**
-   * Defines the horizontal stack used for specifying the inner layout of the Button.
-   */
-  stack?: IStackSlot;
-
   /**
    * Defines the text that is displayed inside the Button.
    */
@@ -206,6 +201,11 @@ export interface IButtonTokens {
    */
   borderWidthFocused?: number | string;
 
+  /**
+   * Defines the spacing between Button children.
+   */
+  childrenGap?: number | string;
+
   /**
    * Defines the text color of elements inside the Button.
    */

diff --git a/packages/experiments/src/components/Button/Button.view.tsx b/packages/experiments/src/components/Button/Button.view.tsx
@@ -1,5 +1,5 @@
 /** @jsx withSlots */
-import { Stack, Text, KeytipData } from 'office-ui-fabric-react';
+import { Text, KeytipData } from 'office-ui-fabric-react';
 import { withSlots, getSlots } from '../../Foundation';
 import { getNativeProps, anchorProperties, buttonProperties } from '../../Utilities';
 import { Icon } from '../../utilities/factoryComponents';
@@ -16,7 +16,6 @@ export const ButtonView: IButtonComponent['view'] = props => {
 
   const Slots = getSlots<IButtonProps, IButtonSlots>(props, {
     root: slotType,
-    stack: Stack,
     icon: Icon,
     content: Text
   });
@@ -44,11 +43,9 @@ export const ButtonView: IButtonComponent['view'] = props => {
       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.icon />
+      <Slots.content />
+      {children}
     </Slots.root>
   );
 

diff --git a/packages/experiments/src/components/Button/CompoundButton.tsx b/packages/experiments/src/components/Button/CompoundButton.tsx
@@ -4,6 +4,7 @@ import { IButtonComponent, IButtonStylesReturnType, IButtonTokenReturnType } fro
 import { IButtonVariantProps } from './ButtonVariants.types';
 import { HighContrastSelector } from '../../Styling';
 import { Text } from 'office-ui-fabric-react';
+import { parseGap } from 'office-ui-fabric-react/lib/components/Stack/StackUtils';
 
 export interface ICompoundButtonProps extends IButtonVariantProps {
   secondaryText?: string;
@@ -62,9 +63,24 @@ const CompoundButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
   const { disabled, primary } = props;
   const { semanticColors } = theme;
 
+  const { rowGap, columnGap } = parseGap(tokens.childrenGap, theme);
+
   return {
     root: {
-      lineHeight: '100%'
+      alignItems: 'flex-start',
+      flexDirection: 'column',
+      lineHeight: '100%',
+
+      selectors: {
+        '> *': {
+          marginLeft: 0,
+          marginTop: `${0.5 * rowGap.value}${rowGap.unit} ${0.5 * columnGap.value}${columnGap.unit}`
+        },
+        '> *:not(:first-child)': {
+          marginLeft: 0,
+          marginTop: `${rowGap.value}${rowGap.unit}`
+        }
+      }
     },
     content: {
       color: disabled ? semanticColors.buttonTextDisabled : primary ? semanticColors.primaryButtonText : semanticColors.buttonText,
@@ -94,22 +110,14 @@ const CompoundButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
 export const CompoundButton: CompoundButtonType = props => {
   const { text, iconProps, secondaryText, ...rest } = props;
 
-  const stackTokens = { childrenGap: 5 };
   const secondaryTextStyles = {
     root: {
       height: 12
     }
   };
 
   return (
-    <Button
-      stack={{ as: 'span', horizontal: false, horizontalAlign: 'start', tokens: stackTokens }}
-      content={text}
-      icon={iconProps}
-      styles={CompoundButtonStyles}
-      tokens={CompoundButtonTokens}
-      {...rest}
-    >
+    <Button content={text} icon={iconProps} styles={CompoundButtonStyles} tokens={CompoundButtonTokens} {...rest}>
       <Text variant="small" styles={secondaryTextStyles}>
         {secondaryText}
       </Text>

diff --git a/packages/experiments/src/components/Button/MenuButton/MenuButton.types.tsx b/packages/experiments/src/components/Button/MenuButton/MenuButton.types.tsx
@@ -1,7 +1,7 @@
-import { IComponent, IComponentStyles, ISlotProp, IStyleableComponentProps } from '../../../Foundation';
+import { IComponent, IComponentStyles, ISlottableProps, ISlotProp, IStyleableComponentProps } from '../../../Foundation';
 import { IContextualMenuSlot, IIconSlot } from '../../../utilities/factoryComponents.types';
 import { IBaseProps } from '../../../Utilities';
-import { IButtonProps, IButtonSlot, IButtonSlots, IButtonTokens, IButtonViewProps, INativeButtonProps } from '../Button.types';
+import { IButton, IButtonProps, IButtonSlot, IButtonSlots, IButtonTokens, IButtonViewProps, INativeButtonProps } from '../Button.types';
 
 /**
  * {@docCategory Button}
@@ -48,19 +48,14 @@ export interface IMenuButtonSlots extends IButtonSlots {
 /**
  * {@docCategory Button}
  */
-export interface IMenuButton {
-  /**
-   * Sets focus to the MenuButton.
-   */
-  focus: () => void;
-}
+export interface IMenuButton extends IButton {}
 
 /**
  * {@docCategory Button}
  */
 export interface IMenuButtonProps
-  extends IMenuButtonSlots,
-    Pick<IButtonProps, 'href' | 'primary' | 'disabled' | 'checked' | 'allowDisabledFocus' | 'ariaLabel' | 'keytipProps'>,
+  extends ISlottableProps<IMenuButtonSlots>,
+    Pick<IButtonProps, 'href' | 'primary' | 'disabled' | 'checked' | 'allowDisabledFocus' | 'ariaLabel' | 'keytipProps' | 'uniqueId'>,
     IStyleableComponentProps<IMenuButtonProps, IMenuButtonTokens, IMenuButtonStyles>,
     IBaseProps<IMenuButton>,
     INativeButtonProps {

diff --git a/packages/experiments/src/components/Button/MenuButton/MenuButton.view.tsx b/packages/experiments/src/components/Button/MenuButton/MenuButton.view.tsx
@@ -31,7 +31,6 @@ export const MenuButtonView: IMenuButtonComponent['view'] = props => {
   const Slots = getSlots<IMenuButtonProps, IMenuButtonSlots>(props, {
     root: 'div',
     button: Button,
-    stack: Stack,
     icon: Icon,
     content: Text,
     menu: ContextualMenu,

diff --git a/packages/experiments/src/components/Button/SplitButton/SplitButton.styles.ts b/packages/experiments/src/components/Button/SplitButton/SplitButton.styles.ts
@@ -74,6 +74,8 @@ export const SplitButtonStyles: ISplitButtonComponent['styles'] = (props, theme,
         borderRadius: tokens.borderRadius,
         boxSizing: 'border-box',
         display: 'inline-flex',
+        flexDirection: 'row',
+        justifyContent: 'stretch',
         zIndex: 1,
 
         selectors: {
@@ -146,6 +148,8 @@ export const SplitButtonStyles: ISplitButtonComponent['styles'] = (props, theme,
       borderRightWidth: 0,
       borderTopWidth: props.primaryActionDisabled ? 0 : tokens.borderWidth,
       boxSizing: 'border-box',
+      display: 'inline-flex',
+      flexDirection: 'column',
       height: 'auto',
       width: 'auto',
 

diff --git a/packages/experiments/src/components/Button/SplitButton/SplitButton.types.tsx b/packages/experiments/src/components/Button/SplitButton/SplitButton.types.tsx
@@ -1,8 +1,8 @@
-import { IStackSlot } from 'office-ui-fabric-react';
 import { IComponent, IComponentStyles, IHTMLSlot, ISlotProp, ISlottableProps, IStyleableComponentProps } from '../../../Foundation';
 import { IBaseProps } from '../../../Utilities';
 import { INativeButtonProps } from '../Button.types';
 import {
+  IMenuButton,
   IMenuButtonProps,
   IMenuButtonSlot,
   IMenuButtonSlots,
@@ -40,7 +40,7 @@ export interface ISplitButtonSlots extends IMenuButtonSlots {
   /**
    * Defines the root slot of the component.
    */
-  root?: IStackSlot;
+  root?: IHTMLSlot;
 
   /**
    * Menu button that is going to be rendered.
@@ -50,7 +50,7 @@ export interface ISplitButtonSlots extends IMenuButtonSlots {
   /**
    * Defines the container for the divider that is used for styling purposes.
    */
-  splitDividerContainer?: IStackSlot;
+  splitDividerContainer?: IHTMLSlot;
 
   /**
    * Defines the divider that separates the left and right parts of a SplitButton.
@@ -61,12 +61,7 @@ export interface ISplitButtonSlots extends IMenuButtonSlots {
 /**
  * {@docCategory Button}
  */
-export interface ISplitButton {
-  /**
-   * Sets focus to the first focus stop of the SplitButton.
-   */
-  focus: () => void;
-}
+export interface ISplitButton extends IMenuButton {}
 
 /**
  * {@docCategory Button}
@@ -84,6 +79,7 @@ export interface ISplitButtonProps
       | 'allowDisabledFocus'
       | 'ariaLabel'
       | 'keytipProps'
+      | 'uniqueId'
       | 'defaultExpanded'
       | 'expanded'
       | 'onMenuDismiss'