Skip to content
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.

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

Refactor: Button #6134

Closed
26 of 37 tasks
micahgodbolt opened this issue Aug 29, 2018 · 2 comments
Closed
26 of 37 tasks

Refactor: Button #6134

micahgodbolt opened this issue Aug 29, 2018 · 2 comments

Comments

@micahgodbolt
Copy link
Member

micahgodbolt commented Aug 29, 2018

Component Details

The implementation of the Button component currently existing in the office-ui-fabric-react package has become more complex and its api more confusing as time has gone by. In an effort to simplify the api we are rewriting Button into a new component that's easier to use and easier to maintain.

In tandem with these efforts, we are taking this opportunity to use all of the latest developments in the new Button component, including:

  • Slots
  • Tokens for styling
  • Foundation library for separating state and view
  • Theming

Steps

Upgrade Plan

Part of upgrading usage of the old Button component to the new one is having a compelling story of how to transition current users seamlessly. In order to do that we need to establish what are we going to do with all existing exports.

Below is a list of all existing exports under the current Button (except the ones tha come from example files) and the actions we're taking for each one:

Component Existing export Action taken PR
ActionButton getStyles Styling in ActionButtonTokens #8645
ActionButton ActionButton Created ActionButton variant #8645
BaseButton IButtonClassNames Styling in BaseButtonStyles and BaseButtonTokens #8679
BaseButton ButtonGlobalClassNames Styling in BaseButtonStyles and BaseButtonTokens #8679
BaseButton getBaseButtonClassNames Styling in BaseButtonStyles and BaseButtonTokens #8679
BaseButton getStyles Styling in BaseButtonStyles and BaseButtonTokens #8679
BaseButton IBaseButtonProps Gotten rid of the props
BaseButton IBaseButtonState Gotten rid of the state
BaseButton BaseButton Created BaseButton variant #8679
Button Button Replaced by new Button
Button IButton See API Review section below
Button IButtonProps See API Review section below
Button ElementType Functionality emulated by IButtonProps
Button ButtonType Functionality emulated by variants
Button IButtonStyles Styling emulated in IButtonSlots and IButtonTokens
ButtonThemes standardStyles Styling in Button.styles.ts
ButtonThemes primaryStyles Styling in Button.styles.ts
CommandBarButton getStyles Styling in CommandBarButtonTokens #8721
CommandBarButton CommandBarButton Created CommandBarButton variant #8721
CommandButton CommandButton Created CommandButton variant #8721
CompoundButton getStyles Styling in CompoundButtonStyles and CompoundButtonTokens #8679
CompoundButton CompoundButton Created CompoundButton variant #8679
DefaultButton getStyles Styling in Button.styles.ts
DefaultButton DefaultButton Created DefaultButton variant #8641
IconButton getStyles Styling in IconButtonTokens #8721
IconButton IconButton Created IconButton variant #8721
MessageBarButton getStyles Styling in MessageBarButtonTokens #8721
MessageBarButton MessageBarButton Created MessageBarButton variant #8721
PrimaryButton PrimaryButton Created PrimaryButton variant #8641
SplitButton ISplitButtonClassNames Created SplitButton variant
SplitButton getClassNames Styling in SplitButtonStyles and SplitButtonTokens
SplitButton getStyles Styling in SplitButtonStyles and SplitButtonTokens

Fluent style parity between old and new Button

For every example, the Buttons above are the old Buttons and the Buttons below are the new ones.

Base Buttons:

image

Default and Primary Buttons:

image

Menu Buttons:

image

Split Buttons:

Slightly different to fix some visual bugs in these buttons:

image

Action Buttons:

image

CommandBar Buttons:

image

Compound Buttons:

image

Icon Buttons:

Slightly different to fix some visual bugs in these buttons:

image

MessageBar Buttons:

image

API Review

As part of the migration from the old Button approach currently in the office-ui-fabric-react package to the new one currently in the experiments package, there needs to be a functional parity between the two components. This is so taht we can have a compelling story when promoting the new Button.

Below is a mapping between the properties of the old Button and the actions we took for each one to transition them to the new Button component:

IButtonProps

Old Button prop Action taken Property transitioned? Breaking change? Codemod created?
href Included in the new Button component as part of IButtonProps Not needed
primary Included in the new Button component as part of IButtonProps Not needed
disabled Included in the new Button component as part of IButtonProps Not needed
ariaLabel Included in the new Button component as part of IButtonProps Not needed
checked Included in the new Button component as part of IButtonProps Not needed
allowDisabledFocus Included in the new Button component as part of IButtonProps Not needed
keytipProps Included in the new Button component as part of IButtonProps Not needed
uniqueId Included in the new Button component as part of IButtonProps Not needed
componentRef Included in the new Button component as part of IBaseProps Not needed
styles Included in the new Button component as part of IBaseProps Not needed
theme Included in the new Button component as part of IBaseProps Not needed
className Included in the new Button component as part of IBaseProps Not needed
text Included in the new Button component as content slot in IButtonSlots
iconProps Included in the new Button component as icon slot in IButtonSlots
menuProps Included in the new Button component as menu slot in IButtonSlots
menuIconProps Included in the new Button component as menuIcon slot in IButtonSlots
onAfterMenuDismiss Included in the new Button component as onMenuDismiss in IButtonViewProps
onRenderChildren Functionality provided by slot approach via root or stack slots
onRenderIcon Functionality provided by slot approach via icon slot
onRenderText Functionality provided by slot approach via content slot
onRenderMenuIcon Functionality provided by slot approach via menuIcon slot
menuAs Functionality provided by slot approach via menu slot
onMenuClick Included in MenuButton variant as onClick in IMenuButtonProps
split Included as new Button variant SplitButton
primaryDisabled Included in SplitButton variant as primaryActionDisabled in ISplitMenuButtonProps
splitButtonAriaLabel Included inSplitButton variant as secondaryAriaLabel in ISplitMenuButtonProps
buttonType Removing as already deprecated
rootProps Removing as already deprecated
onRenderMenu Removing as already deprecated in favor of menuAs
toggled Removing as already deprecated in favor of checked
description Removing as already deprecated in favor of secondaryText
ariaHidden Removing in favor of native prop
getClassNames Removing given new styling and theming approach
getSplitButtonClassNames Removing given new styling and theming approach
secondaryText Removing given that this multiline functionality is provided with new slot approach
toggle Removing in favor of a stateless approach towards toggle buttons.
onRenderDescription Removing given that secondaryText is being deprecated
onRenderAriaDescription Removing given that secondaryText is being deprecated
ariaDescription Removing given that secondaryText is being deprecated
persistMenu Removing controlled menu behavior in new Button
data Removing in new Button
menuTriggerKeyCode TBD

IButton

Old Button prop Action to take Property transitioned Breaking change? Codemod created?
focus Included in the new Button component as part of IButton Not needed
dismissMenu Removing controlled menu behavior in new Button
openMenu Removing controlled menu behavior in new Button

Component Documentation

Imports

TBD

Exports/ Component Breakdown

TBD

Intended Package

Initially in @uifabric/experiments while we drive the changes to have the correct api, but with the intention of moving it to office-ui-fabric-react after that is done.

Code mockup/example

TBD

Button Types

Button

IButtonSlots

Name Type Description
root IHTMLElementSlot<Button> Defines the root slot of the component.
stack IStackSlot Defines the horizontal stack used for specifying the inner layout of the Button.
content ITextSlot Defines the text that is displayed inside the Button.
icon IIconSlot Defines the icon that is displayed next to the text inside the Button.

IButtonProps

Name Type Default Value Description
href string Defines an href reference that, if provided, will make this component render as an anchor.
primary boolean false Defines whether the visual representation of the Button should be emphasized.
circular boolean false Defines whether the Button should be circular. In general, circular Buttons should not specify the menu and container slots.
disabled boolean false Defines whether the Button is disabled.
onClick (ev: React.MouseEvent<HTMLElement>) => void Defines an event callback that is triggered when the Button is clicked.

IButtonViewProps

IButtonTokens

Name Type
backgroundColor strong
backgroundColorHovered string
backgroundColorPressed string
color string
colorFocused string
colorHovered string
colorPressed string
borderColor string
borderColorHovered string
borderColorPressed string
iconColor string
iconColorHovered string
iconColorPressed string
outlineColor string
borderRadius number | string
borderWidth number | string
contentPadding number | string
contentPaddingFocused number | string
textFamily string
textSize number | string
textWeight IFontWeight
width number | string
height number | string
iconSize number | string
iconWeight number
lineHeight number | string
minWidth number | string
minHeight number | string
backgroundClip IRawStyleBase['backgroundClip']

MenuButton

IMenuButtonSlots

Name Type Description
root IHTMLElementSlot<div> Defines the root slot of the component.
button IButtonSlot Defines the button that is going to be rendered.
menu IContextualMenuSlot Defines the contextual menu that appears when you click on the MenuButton.
menuIcon IIconSlot Defines the icon that is displayed next to the text inside the MenuButton.

IMenuButtonProps

Name Type Default Value Description
defaultExpanded boolean false Defines the inital expanded state of the MenuButton. If you want the MenuButton to maintain its own state, use this. Otherwise, refer to expanded.
expanded boolean defaultExpanded Defines whether the MenuButton is in an expanded state.
onKeyDown (ev: React.KeyboardEvent<HTMLElement>) => void Defines an event callback that is triggered when a keypress is made with the focus on a MenuButton.

IMenuButtonViewProps

Name Type Default Value Description
onMenuDismiss () => void Defines a callback that runs after the MenuButton's contextual menu has been closed (removed from the DOM).
menuTarget HTMLElement | undefined undefined Defines the target that the contextual menu uses to position itself.

IMenuButtonTokens

SplitButton

ISplitButtonSlots

Name Type Description
root IStackSlot Defines the root slot of the component.
menuButton IMenuButtonSlot Menu button that is going to be rendered.
splitDivider IHTMLSlot Defines the divider that separates the left and right parts of a SplitButton.

ISplitButtonProps

Name Type Default Value Description
primaryActionDisabled boolean false Defines whether the first action of the SplitButton is disabled.

ISplitButtonViewProps

Name Type Default Value Description
onSecondaryActionClick (ev: React.MouseEvent<HTMLElement>) => void Defines whether the first action of the SplitButton is disabled.

ISplitButtonTokens

Bundle Size Improvements

Old Button (Stat: 564.5KB, Parsed: 175.3KB, Gzipped: 47.35KB):
image

New Button (Stat: 325.08KB, Parsed: 88.83KB, Gzipped: 26.6KB):
image

Perf Comparison

The scenarios render the components 5000 times. Each scenario is run 10 times to get an average number and account for variants.

DefaultButton:

Scenario Run 1 Run 2 Run 3 Run 4 Run 5 Run 6 Run 7 Run 8 Run 9 Run 10 Average
Old Button 498 545 521 536 548 544 509 539 536 553 538.9
Before Stack removal 1451 1515 1473 1454 1498 1439 1531 1498 1604 1508 1497.1
After Stack removal 1152 1039 1100 1132 1143 1169 1099 1165 1156 1184 1133.9

210.41% worse than old Button.
24.26% better after Stack removal.

PrimaryButton:

Scenario Run 1 Run 2 Run 3 Run 4 Run 5 Run 6 Run 7 Run 8 Run 9 Run 10 Average
Old Button 574 565 581 627 604 564 594 610 612 614 594.5
Before Stack removal 1459 1541 1427 1550 1521 1492 1492 1546 1630 1534 1519.2
After Stack removal 1093 1162 1139 1207 1182 1201 1167 1166 1154 1200 1167.1

196.32% worse than old Button.
23.18% better after Stack removal.

MenuButton:

Scenario Run 1 Run 2 Run 3 Run 4 Run 5 Run 6 Run 7 Run 8 Run 9 Run 10 Average
Old Button 924 952 920 899 883 910 959 905 879 924 915.5
Before Stack removal 2392 2413 2514 2494 2434 2448 2406 2455 2537 2421 2451.4
After Stack removal 2150 2127 2137 2128 2118 2146 2197 2203 2206 2161 2157.3

235.64% worse than old Button.
12.00% better after Stack removal.

SplitButton:

Scenario Run 1 Run 2 Run 3 Run 4 Run 5 Run 6 Run 7 Run 8 Run 9 Run 10 Average
Old Button 1645 1722 1697 1698 1702 1651 1668 1774 1616 1646 1681.9
Before Stack removal 5088 5024 4988 5015 5102 5026 5031 5117 5110 5098 5059.9
After Stack removal 3810 3847 3910 3771 3810 3656 3709 3724 3847 3803 3788.7

225.26% worse than old Button.
25.12% better after Stack removal.

Design Assets

Component Ownership

  • @khmakoto who is contributing to the rewrite and doing the api review against the old Button component already in office-ui-fabric-react.

Deadlines

After 7.0 release.

Promotion Checklist

@micahgodbolt
Copy link
Member Author

Closing down #3308 and moving the new button experiment to here.

@micahgodbolt micahgodbolt changed the title Button rewrite experiment Rewrite Button in Experiments Aug 29, 2018
@dzearing dzearing assigned khmakoto and unassigned dzearing Jan 7, 2019
@khmakoto khmakoto changed the title Rewrite Button in Experiments New Component: Button rewrite Jan 10, 2019
@dzearing dzearing added this to the February 2019 milestone Jan 15, 2019
@dzearing dzearing modified the milestones: February 2019, March 2019 Feb 13, 2019
@khmakoto khmakoto modified the milestones: March 2019, April 2019 Apr 1, 2019
@khmakoto khmakoto modified the milestones: April 2019, May 2019 May 9, 2019
@JasonGore JasonGore changed the title New Component: Button rewrite Refactor: Button May 16, 2019
@micahgodbolt micahgodbolt removed this from the May 2019 milestone Aug 19, 2019
@JustSlone
Copy link
Collaborator

This is covered via button convergence, not needed anymore

Check out the new button here: https://github.com/microsoft/fluentui/tree/master/packages/react-button

@microsoft microsoft locked as resolved and limited conversation to collaborators Feb 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants