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

Button: Migration for text, iconProps, menuProps and split deprecated props #9354

Closed
wants to merge 3 commits into from

Conversation

khmakoto
Copy link
Member

@khmakoto khmakoto commented Jun 5, 2019

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

This PR creates code mods for some Button props to aid in the transition of people using the old Button in Fabric 7. The code mods do the following things:

  • Rename the deprecated text prop to content.
  • Rename the deprecated iconProps prop to icon.
  • Rename the deprecated menuProps prop to menu.
  • For Buttons that have the menuProps prop, rename Button to MenuButton and add MenuButton as an import.
  • For Buttons that have the split prop, rename Button to SplitButton, remove the split prop and add SplitButton as an import.

In order to do this two custom reusable code mods have been created, one to rename a JSX node to a new name if it has a prop and another one to remove a prop from a JSX node.

index.ts has also been modified to include accept an array of code mods that are applied in sequence instead of only accepting one code mod at a time.

With these code mods applied, the file:

import { Button } from '@uifabric/experiments';
import { Fabric } from 'office-ui-fabric-react';

const menuProps = {
  items: [
    {
      key: 'a',
      name: 'Item a'
    },
    {
      key: 'b',
      name: 'Item b'
    }
  ]
};

class ButtonExample extends React.Component<{}, {}> {
  public render(): JSX.Element {
    return (
      <Fabric className={wrapperClassName}>
        <Button text="This is a button" iconProps={{ iconName: 'Emoji2' }} />
        <Button text="This is a button" iconProps={{ iconName: 'Emoji2' }} menuProps={menuProps} />
        <Button split text="This is a button" iconProps={{ iconName: 'Emoji2' }} menuProps={menuProps} />
      </Fabric>
    );
  }
}

Is transformed into:

import { Button } from '@uifabric/experiments';
import { MenuButton } from '@uifabric/experiments';
import { SplitButton } from '@uifabric/experiments';
import { Fabric } from 'office-ui-fabric-react';

const menuProps = {
  items: [
    {
      key: 'a',
      name: 'Item a'
    },
    {
      key: 'b',
      name: 'Item b'
    }
  ]
};

class ButtonExample extends React.Component<{}, {}> {
  public render(): JSX.Element {
    return (
      <Fabric className={wrapperClassName}>
        <Button content=\\"This is a button\\" icon={{ iconName: 'Emoji2' }} />
        <MenuButton content=\\"This is a button\\" icon={{ iconName: 'Emoji2' }} menu={menuProps} />
        <SplitButton content=\\"This is a button\\" icon={{ iconName: 'Emoji2' }} menu={menuProps} />
      </Fabric>
    );
  }
}

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

buttonToMenuButtonMigration,
splitButtonSplitPropRemovalMigration
],
'button.tsx'
Copy link
Member Author

Choose a reason for hiding this comment

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

@jdhuntington Here is the change I made for it to work on Windows. Based on the other examples, this would be path.join('7.0.0', 'button.tsx') instead. Changing the other examples also makes them work on Windows.

@msft-github-bot
Copy link
Contributor

Component perf results:

Scenario Target branch avg total (ms) PR avg total (ms) Target branch avg per item (ms) PR avg per item (ms) Is significant change Is regression
PrimaryButton 82.757 73.036 0.828 0.730 true false
BaseButton 31.785 28.129 0.318 0.281 true false
NewButton 108.044 ... 1.080 ... ... ...
button 5.907 5.582 0.059 0.056 false false
DetailsRows without styles 180.472 175.211 1.805 1.752 false false
DetailsRows 201.118 201.167 2.011 2.012 false false
Toggles 52.236 50.225 0.522 0.502 false false
NewToggle 67.870 60.839 0.679 0.608 true false
DocumentCardTitle with truncation 26.014 25.422 0.260 0.254 false false
DefaultButton ... 69.535 ... 0.695 ... ...
MenuButton ... 87.439 ... 0.874 ... ...
SplitButton ... 157.505 ... 1.575 ... ...
NewDefaultButton ... 76.569 ... 0.766 ... ...
NewPrimaryButton ... 74.287 ... 0.743 ... ...
NewMenuButton ... 140.577 ... 1.406 ... ...
NewSplitButton ... 269.460 ... 2.695 ... ...

@micahgodbolt micahgodbolt mentioned this pull request Jun 13, 2019
37 tasks
@khmakoto khmakoto closed this Jun 13, 2019
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
@khmakoto khmakoto deleted the buttonCodeMods branch April 7, 2021 21:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants