feat(icon): convert icon slots to ShorthandValue<BoxProps> #12489

mnajdova · 2020-03-31T12:22:11Z

This PR changes all icon slots inside @fluentui/react-northstar to be shorthands of Box. All icon shorthands are replaced with icon components from the @fluentui/react-icons-northstar package.

This PR will not remove the Icon comeponent and will not change the Icon docs. Those will be implemented in follow up PRs.

There are few broken screeners mainly because of font -> svg icon changes

TODOs:

add changelog entry
fix icon usages in broken screener tests

BREAKING CHANGES

The icon prop in the following components was changed from ShorthandValue<IconProps> to ShorthandValue<BoxProps>:

Attachment
Button
DropdownSelectedItem
Input
Label
MenuItem
Reaction
Status
ToolbarItem
ToolbarMenuItem

The conversion looks like this:

<Button icon={{name: 'circle', outline: true }} /> => <Button icon={<CircleIcon />} />

Codemods prepared for the migration: https://github.com/joschect/ts-morphin-migration/tree/iconbranch

Note:
The codemods works only known component icon props, where the value string or object literal on the components.

Steps for migration:

Change open icon jsx tags to self closing: <Icon ..></Icon> => <Icon ... />
Run the codemods
There will be more occurences for cases where some prop/objects are used and spreaded across components. Here are few useful searches that can be used for catching these (with Match case & Regexp):

icon=\{[^<]

icon: \{

icon: "(\w+)"
icon: "(\w+)-(\w+)"
icon: "(\w+)-(\w+)-(\w+)"

<Icon.*name

-updated button props and examples

-fixed inconsistencies

…ents-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

-added Icon suffix in name test

…-updates-part-1

mnajdova · 2020-04-03T10:11:46Z

packages/fluentui/react-northstar/src/components/Dropdown/DropdownSelectedItem.tsx

@@ -92,7 +92,8 @@ class DropdownSelectedItem extends UIComponent<WithAsProp<DropdownSelectedItemPr
  };

  static defaultProps = {
-    icon: 'close',
+    // TODO: fix me
+    icon: <CloseIcon />,


Will be fixed in a follow up PR to avoid unrelated changes

assuncaocharles · 2020-04-03T10:21:49Z

packages/fluentui/docs/src/components/ComponentDoc/ComponentControls/ComponentControls.tsx

@@ -69,7 +69,7 @@ const ComponentControls: React.FC<ComponentControlsProps> = props => {
            aria-label={toolbarAriaLabel || null}
            items={[
              {
-                icon: { name: 'code', style: { width: '20px', height: '20px' } },
+                icon: <Icon name={'code'} {...{ style: { width: '20px', height: '20px' } }} />,


Any reason to actually use spread for style instead of direct using the prop/attribute?

icon: <Icon name="code" style={{ width: '20px', height: '20px' }} />,

This was auto generated by the codemods, which basically remove the name prop and spread the rest of the object on the component.

assuncaocharles · 2020-04-03T10:30:31Z

packages/fluentui/docs/src/components/GuidesNavigationFooter.tsx

+        <Button
+          as={Link}
+          content={next.name}
+          icon={<Icon name="arrow right" />}


Should it be <ArrowRightIcon /> ?

This is a font icon, we don't have mapping for an svg icon

You mean when using icon in the button is font Icon or the arrow left itself? I got confused because PopupExamplePointing.shorthand.tsx it was using <ArrowRightIcon />

It is a bit confusion yeah. Basically name="arrow-left" can be transform to <ArrowLeftIcon />, but here the name is arrow left which is a different font icon. Anyway, as it is not in an example we can change it to <ArrowRightIcon />, but my point was that it is a different icon.

packages/fluentui/react-northstar/src/themes/teams/components/Status/statusStyles.ts

packages/fluentui/react-northstar/src/themes/teams/components/Menu/menuItemStyles.ts

mnajdova · 2020-04-03T12:52:05Z

packages/fluentui/react-northstar/src/components/Toolbar/Toolbar.tsx

@@ -447,7 +448,8 @@ class Toolbar extends UIComponent<WithAsProp<ToolbarProps>> {
      <Ref innerRef={this.overflowItemRef}>
        {ToolbarItem.create(overflowItem, {
          defaultProps: () => ({
-            icon: { name: 'more', outline: true },
+            // TODO: ups
+            icon: <MoreIcon {...{ outline: true }} />,


Will be fixed after these changes are merged

mnajdova · 2020-04-03T12:52:26Z

packages/fluentui/react-northstar/src/components/Status/Status.tsx

        styles: resolvedStyles.icon,
-        xSpacing: 'none',


This is default no need for overriding styles

mnajdova · 2020-04-03T12:54:32Z

packages/fluentui/react-northstar/src/components/Label/Label.tsx

@@ -105,12 +104,6 @@ const Label: React.FC<WithAsProp<LabelProps>> & FluentComponentStaticProps = pro
    rtl: context.rtl,
  });

-  const handleIconOverrides = (predefinedProps: IconProps) => ({


No need for this as xSpacing: undefined == 'none' is default

mnajdova · 2020-04-03T12:54:49Z

packages/fluentui/react-northstar/src/components/Input/Input.tsx

    const { clearable, icon } = this.props;
    const { value } = this.state;

    if (clearable && (value as string).length !== 0) {
-      return { name: '' };


mnajdova · 2020-04-03T12:56:04Z

packages/fluentui/react-northstar/src/components/Button/Button.tsx

      defaultProps: () =>
        getA11Props('icon', {
          styles: resolvedStyles.icon,
-          xSpacing: !content ? 'none' : iconPosition === 'after' ? 'before' : 'after',


Moved to styles

# Conflicts: # packages/fluentui/react-northstar/src/components/Alert/Alert.tsx

packages/fluentui/docs/src/prototypes/chatPane/services/messageFactoryMock.tsx

…eFactoryMock.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>

…#12489) * wip * -fixed imports * -added icons * -reverted unintended changes * -reverted unintended changes * -reverted unintended changes * -reverted unintended changes * -fixed imports * -added redPath * -updated icon prop to box shorthand -updated button props and examples * -fixed jest config * -fixed references * -added components tests -fixed inconsistencies * -ts-morphin experiments * -perf experiments * -perf experiments * Update packages/fluentui/react-icons-northstar/test/components/components-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-icons-northstar/test/components/components-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-icons-northstar/test/components/components-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * -added packages * -fixed tests * -added Icon suffix * -reverted perf test changes * -removed author from package.json * -changed some icon names to reflect the previous names -added Icon suffix in name test * -removed comments * -fixed names * ts-morphin changes * -more ts-morphin changes * -more ts-morphin changes * -updated styles * -updated menuItemStyles * -fixed renderConfig * -more ts morphin changes * -more ts morphin changes (collection components) * -added as: 'span' in MenuItem * -more changes f+r * -more changes ts-morphin * -more changes ts-morphin * -more changes ts-morphin * -more changes ts-morphin * -more changes ts-morphin * -fixes * -more changes f & r * -more changes f & r * -more changes f & r * -more changes f & r * -more changes f & r * -fixes in status styles * -fixes in components still dependent on icons * -fixes * -fixes * -fixed some styles * -fixed some styles and examples * -test button styles * -added variables * -added variables * -fixed toolbar menu item icon spacing * -removed comment * -fixed tests * Update packages/fluentui/docs/src/prototypes/chatPane/services/messageFactoryMock.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * Update packages/fluentui/docs/src/prototypes/chatPane/services/messageFactoryMock.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * -final fixes * -fixed tests * -fixed tests Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com>

mnajdova and others added 30 commits March 27, 2020 12:02

wip

a957127

-fixed imports

55438f6

-added icons

cd712e6

-reverted unintended changes

57041cf

-reverted unintended changes

7bbfd49

-reverted unintended changes

e139432

-reverted unintended changes

d752162

-fixed imports

16152e0

-added redPath

edf8a20

Merge branch 'master' into feat/add-icons-northstar-package

ee2842d

-updated icon prop to box shorthand

5d1b613

-updated button props and examples

-fixed jest config

14c57a2

-fixed references

d07e9b2

-added components tests

aa65178

-fixed inconsistencies

-ts-morphin experiments

e7595cb

-perf experiments

9bda8c2

-perf experiments

8aaf7ba

Update packages/fluentui/react-icons-northstar/test/components/compon…

850bcc4

…ents-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Update packages/fluentui/react-icons-northstar/test/components/compon…

ea886c8

…ents-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Update packages/fluentui/react-icons-northstar/test/components/compon…

962d0f8

…ents-test.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

-added packages

2d5ecde

-fixed tests

c20341b

-added Icon suffix

80a53c3

-reverted perf test changes

446d64b

-removed author from package.json

cc85159

-changed some icon names to reflect the previous names

34a4a78

-added Icon suffix in name test

-removed comments

e96f5f8

Merge branch 'feat/add-icons-northstar-package' into feat/icons-props…

7e2e765

…-updates-part-1

-fixed names

7105d58

ts-morphin changes

8fa1aa9

mnajdova commented Apr 3, 2020

View reviewed changes

mnajdova changed the title ~~[WIP] feat(icon): convert icon slots to ShorthandValue<BoxProps>~~ feat(icon): convert icon slots to ShorthandValue<BoxProps> Apr 3, 2020

assuncaocharles reviewed Apr 3, 2020

View reviewed changes

pompomon reviewed Apr 3, 2020

View reviewed changes

packages/fluentui/react-northstar/src/themes/teams/components/Status/statusStyles.ts Outdated Show resolved Hide resolved

pompomon reviewed Apr 3, 2020

View reviewed changes

packages/fluentui/react-northstar/src/themes/teams/components/Menu/menuItemStyles.ts Outdated Show resolved Hide resolved

mnajdova added 3 commits April 3, 2020 14:36

-added variables

6dce2d5

-added variables

cb658cd

-fixed toolbar menu item icon spacing

b268ef3

mnajdova commented Apr 3, 2020

View reviewed changes

-removed comment

569b3d5

mnajdova commented Apr 3, 2020

View reviewed changes

mnajdova added 2 commits April 3, 2020 16:10

Merge branch 'master' into feat/icons-props-updates-part-1

710a806

# Conflicts: # packages/fluentui/react-northstar/src/components/Alert/Alert.tsx

-fixed tests

5807f62

pompomon reviewed Apr 3, 2020

View reviewed changes

packages/fluentui/docs/src/prototypes/chatPane/services/messageFactoryMock.tsx Outdated Show resolved Hide resolved

pompomon reviewed Apr 3, 2020

View reviewed changes

packages/fluentui/docs/src/prototypes/chatPane/services/messageFactoryMock.tsx Outdated Show resolved Hide resolved

mnajdova and others added 2 commits April 3, 2020 17:06

Update packages/fluentui/docs/src/prototypes/chatPane/services/messag…

45b1860

…eFactoryMock.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>

Update packages/fluentui/docs/src/prototypes/chatPane/services/messag…

8253f23

…eFactoryMock.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>

pompomon approved these changes Apr 3, 2020

View reviewed changes

mnajdova added 4 commits April 6, 2020 07:47

Merge branch 'master' into feat/icons-props-updates-part-1

f774a88

-final fixes

baca0bb

-fixed tests

ba5e83b

-fixed tests

8e0106f

mnajdova merged commit 908444b into microsoft:master Apr 6, 2020

mnajdova mentioned this pull request Apr 6, 2020

feat(icons): remove Icon component and update docs #12549

Merged

mnajdova mentioned this pull request Apr 14, 2020

RFC(icon): update icon slots #12389

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(icon): convert icon slots to ShorthandValue<BoxProps> #12489

feat(icon): convert icon slots to ShorthandValue<BoxProps> #12489

mnajdova commented Mar 31, 2020 •

edited

mnajdova Apr 3, 2020

assuncaocharles Apr 3, 2020

mnajdova Apr 3, 2020

assuncaocharles Apr 3, 2020 •

edited

mnajdova Apr 3, 2020

assuncaocharles Apr 3, 2020

mnajdova Apr 3, 2020 •

edited

mnajdova Apr 3, 2020

mnajdova Apr 3, 2020

mnajdova Apr 3, 2020

mnajdova Apr 3, 2020

mnajdova Apr 3, 2020

feat(icon): convert icon slots to ShorthandValue<BoxProps> #12489

feat(icon): convert icon slots to ShorthandValue<BoxProps> #12489

Conversation

mnajdova commented Mar 31, 2020 • edited

BREAKING CHANGES

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

assuncaocharles Apr 3, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova Apr 3, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova commented Mar 31, 2020 •

edited

assuncaocharles Apr 3, 2020 •

edited

mnajdova Apr 3, 2020 •

edited