chore: Cleaning up tokens in Button components so they better adhere …

…to the design spec (#24732) * chore: Cleaning up tokens in Button components so they better adhere to the design spec. * Adding change file. * Applying latest changes from design docs. * Fixing HC issues. * Fixing HC issues. * Fixing HC issues. * Adding proper icon to Button vr stories. Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>
microsoft · Sep 12, 2022 · ef3ea23 · ef3ea23
1 parent 83b033d
commit ef3ea23
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 68 deletions.
diff --git a/apps/vr-tests-react-components/src/stories/Button.stories.tsx b/apps/vr-tests-react-components/src/stories/Button.stories.tsx
@@ -2,8 +2,11 @@ import { storiesOf } from '@storybook/react';
 import * as React from 'react';
 import Screener from 'screener-storybook/src/screener';
 import { Button, CompoundButton, ToggleButton, MenuButton } from '@fluentui/react-button';
+import { bundleIcon, CalendarMonthFilled, CalendarMonthRegular } from '@fluentui/react-icons';
 import { makeStyles } from '@griffel/react';
 
+const CalendarMonth = bundleIcon(CalendarMonthFilled, CalendarMonthRegular);
+
 const steps = new Screener.Steps()
   .snapshot('default', { cropTo: '.testWrapper' })
   .hover('#button-id')
@@ -125,43 +128,43 @@ storiesOf('Button Converged', module)
     { includeHighContrast: true, includeDarkMode: true },
   )
   .addStory('Size small', () => (
-    <Button id={buttonId} icon="X" size="small">
+    <Button id={buttonId} icon={<CalendarMonth />} size="small">
       Hello, world
     </Button>
   ))
   .addStory('Size large', () => (
-    <Button id={buttonId} icon="X" size="large">
+    <Button id={buttonId} icon={<CalendarMonth />} size="large">
       Hello, world
     </Button>
   ))
   .addStory('Size small - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <Button id={buttonId} className={styles.longText} icon="X" size="small">
+      <Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
         Long text wraps after it hits the max width of the component
       </Button>
     );
   })
   .addStory('Size medium - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <Button id={buttonId} className={styles.longText} icon="X" size="medium">
+      <Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
         Long text wraps after it hits the max width of the component
       </Button>
     );
   })
   .addStory('Size large - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <Button id={buttonId} className={styles.longText} icon="X" size="large">
+      <Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
         Long text wraps after it hits the max width of the component
       </Button>
     );
   })
   .addStory(
     'With icon before content',
     () => (
-      <Button id={buttonId} icon="X">
+      <Button id={buttonId} icon={<CalendarMonth />}>
         Hello, world
       </Button>
     ),
@@ -172,14 +175,14 @@ storiesOf('Button Converged', module)
   .addStory(
     'With icon after content',
     () => (
-      <Button id={buttonId} icon="X" iconPosition="after">
+      <Button id={buttonId} icon={<CalendarMonth />} iconPosition="after">
         Hello, world
       </Button>
     ),
     { includeRtl: true },
   )
-  .addStory('Icon only', () => <Button id={buttonId} icon="X" />)
-  .addStory('Circular and icon only', () => <Button id={buttonId} shape="circular" icon="X" />, {
+  .addStory('Icon only', () => <Button id={buttonId} icon={<CalendarMonth />} />)
+  .addStory('Circular and icon only', () => <Button id={buttonId} shape="circular" icon={<CalendarMonth />} />, {
     includeRtl: true,
   });
 
@@ -210,7 +213,7 @@ storiesOf('CompoundButton Converged', module)
   .addStory(
     'With icon before content',
     () => (
-      <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X">
+      <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />}>
         Hello, world
       </CompoundButton>
     ),
@@ -219,7 +222,12 @@ storiesOf('CompoundButton Converged', module)
   .addStory(
     'With icon after content',
     () => (
-      <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" iconPosition="after">
+      <CompoundButton
+        id={buttonId}
+        secondaryContent="This is some secondary text"
+        icon={<CalendarMonth />}
+        iconPosition="after"
+      >
         Hello, world
       </CompoundButton>
     ),
@@ -279,12 +287,12 @@ storiesOf('CompoundButton Converged', module)
     </CompoundButton>
   ))
   .addStory('Size small', () => (
-    <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" size="small">
+    <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />} size="small">
       Hello, world
     </CompoundButton>
   ))
   .addStory('Size large', () => (
-    <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" size="large">
+    <CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />} size="large">
       Hello, world
     </CompoundButton>
   ))
@@ -295,7 +303,7 @@ storiesOf('CompoundButton Converged', module)
         id={buttonId}
         className={styles.longText}
         secondaryContent="This is some secondary text"
-        icon="X"
+        icon={<CalendarMonth />}
         size="small"
       >
         Long text wraps after it hits the max width of the component
@@ -309,7 +317,7 @@ storiesOf('CompoundButton Converged', module)
         id={buttonId}
         className={styles.longText}
         secondaryContent="This is some secondary text"
-        icon="X"
+        icon={<CalendarMonth />}
         size="medium"
       >
         Long text wraps after it hits the max width of the component
@@ -323,17 +331,21 @@ storiesOf('CompoundButton Converged', module)
         id={buttonId}
         className={styles.longText}
         secondaryContent="This is some secondary text"
-        icon="X"
+        icon={<CalendarMonth />}
         size="large"
       >
         Long text wraps after it hits the max width of the component
       </CompoundButton>
     );
   })
-  .addStory('Icon only', () => <CompoundButton id={buttonId} icon="X" />)
-  .addStory('Circular and icon only', () => <CompoundButton id={buttonId} shape="circular" icon="X" />, {
-    includeRtl: true,
-  });
+  .addStory('Icon only', () => <CompoundButton id={buttonId} icon={<CalendarMonth />} />)
+  .addStory(
+    'Circular and icon only',
+    () => <CompoundButton id={buttonId} shape="circular" icon={<CalendarMonth />} />,
+    {
+      includeRtl: true,
+    },
+  );
 
 storiesOf('ToggleButton Converged', module)
   .addDecorator(story => <Screener steps={steps}>{story()}</Screener>)
@@ -395,51 +407,51 @@ storiesOf('ToggleButton Converged', module)
     </ToggleButton>
   ))
   .addStory('Size small', () => (
-    <ToggleButton id={buttonId} icon="X" size="small">
+    <ToggleButton id={buttonId} icon={<CalendarMonth />} size="small">
       Hello, world
     </ToggleButton>
   ))
   .addStory('Size large', () => (
-    <ToggleButton id={buttonId} icon="X" size="large">
+    <ToggleButton id={buttonId} icon={<CalendarMonth />} size="large">
       Hello, world
     </ToggleButton>
   ))
   .addStory('Size small - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <ToggleButton id={buttonId} className={styles.longText} icon="X" size="small">
+      <ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
         Long text wraps after it hits the max width of the component
       </ToggleButton>
     );
   })
   .addStory('Size medium - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <ToggleButton id={buttonId} className={styles.longText} icon="X" size="medium">
+      <ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
         Long text wraps after it hits the max width of the component
       </ToggleButton>
     );
   })
   .addStory('Size large - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <ToggleButton id={buttonId} className={styles.longText} icon="X" size="large">
+      <ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
         Long text wraps after it hits the max width of the component
       </ToggleButton>
     );
   })
   .addStory('With icon before content', () => (
-    <ToggleButton id={buttonId} icon="X">
+    <ToggleButton id={buttonId} icon={<CalendarMonth />}>
       Hello, world
     </ToggleButton>
   ))
   .addStory('With icon after content', () => (
-    <ToggleButton id={buttonId} icon="X" iconPosition="after">
+    <ToggleButton id={buttonId} icon={<CalendarMonth />} iconPosition="after">
       Hello, world
     </ToggleButton>
   ))
-  .addStory('Icon only', () => <ToggleButton id={buttonId} icon="X" />)
-  .addStory('Circular and icon only', () => <ToggleButton id={buttonId} shape="circular" icon="X" />)
+  .addStory('Icon only', () => <ToggleButton id={buttonId} icon={<CalendarMonth />} />)
+  .addStory('Circular and icon only', () => <ToggleButton id={buttonId} shape="circular" icon={<CalendarMonth />} />)
   .addStory('Checked', () => (
     <ToggleButton id={buttonId} checked>
       Hello, world
@@ -515,43 +527,43 @@ storiesOf('MenuButton Converged', module)
     </MenuButton>
   ))
   .addStory('Size small', () => (
-    <MenuButton id={buttonId} icon="X" size="small">
+    <MenuButton id={buttonId} icon={<CalendarMonth />} size="small">
       Hello, world
     </MenuButton>
   ))
   .addStory('Size large', () => (
-    <MenuButton id={buttonId} icon="X" size="large">
+    <MenuButton id={buttonId} icon={<CalendarMonth />} size="large">
       Hello, world
     </MenuButton>
   ))
   .addStory('Size small - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <MenuButton id={buttonId} className={styles.longText} icon="X" size="small">
+      <MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
         Long text wraps after it hits the max width of the component
       </MenuButton>
     );
   })
   .addStory('Size medium - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <MenuButton id={buttonId} className={styles.longText} icon="X" size="medium">
+      <MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
         Long text wraps after it hits the max width of the component
       </MenuButton>
     );
   })
   .addStory('Size large - with long text wrapping', () => {
     const styles = useStyles();
     return (
-      <MenuButton id={buttonId} className={styles.longText} icon="X" size="large">
+      <MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
         Long text wraps after it hits the max width of the component
       </MenuButton>
     );
   })
   .addStory('With icon', () => (
-    <MenuButton id={buttonId} icon="X">
+    <MenuButton id={buttonId} icon={<CalendarMonth />}>
       Hello, world
     </MenuButton>
   ))
-  .addStory('Icon only', () => <MenuButton id={buttonId} icon="X" />)
-  .addStory('Circular and icon only', () => <MenuButton id={buttonId} shape="circular" icon="X" />);
+  .addStory('Icon only', () => <MenuButton id={buttonId} icon={<CalendarMonth />} />)
+  .addStory('Circular and icon only', () => <MenuButton id={buttonId} shape="circular" icon={<CalendarMonth />} />);
diff --git a/change/@fluentui-react-button-2bff9178-dae7-48c6-9b2e-4e5effb0d484.json b/change/@fluentui-react-button-2bff9178-dae7-48c6-9b2e-4e5effb0d484.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "chore: Cleaning up tokens in Button components so they better adhere to the design spec.",
+  "packageName": "@fluentui/react-button",
+  "email": "humberto_makoto@hotmail.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-components/react-button/src/components/Button/useButtonStyles.ts b/packages/react-components/react-button/src/components/Button/useButtonStyles.ts
@@ -129,13 +129,13 @@ const useRootStyles = makeStyles({
     ':hover': {
       backgroundColor: tokens.colorSubtleBackgroundHover,
       ...shorthands.borderColor('transparent'),
-      color: tokens.colorNeutralForeground2BrandHover,
+      color: tokens.colorNeutralForeground2Hover,
     },
 
     ':hover:active': {
       backgroundColor: tokens.colorSubtleBackgroundPressed,
       ...shorthands.borderColor('transparent'),
-      color: tokens.colorNeutralForeground2BrandPressed,
+      color: tokens.colorNeutralForeground2Pressed,
     },
   },
   transparent: {
@@ -270,11 +270,11 @@ const useRootDisabledStyles = makeStyles({
     backgroundColor: tokens.colorTransparentBackground,
 
     ':hover': {
-      backgroundColor: tokens.colorTransparentBackgroundHover,
+      backgroundColor: tokens.colorTransparentBackground,
     },
 
     ':hover:active': {
-      backgroundColor: tokens.colorTransparentBackgroundPressed,
+      backgroundColor: tokens.colorTransparentBackground,
     },
   },
   primary: {
@@ -292,39 +292,39 @@ const useRootDisabledStyles = makeStyles({
     /* The secondary styles are exactly the same as the base styles. */
   },
   subtle: {
-    backgroundColor: 'transparent',
+    backgroundColor: tokens.colorTransparentBackground,
     ...shorthands.borderColor('transparent'),
 
     ':hover': {
-      backgroundColor: 'transparent',
+      backgroundColor: tokens.colorTransparentBackground,
       ...shorthands.borderColor('transparent'),
     },
 
     ':hover:active': {
-      backgroundColor: 'transparent',
+      backgroundColor: tokens.colorTransparentBackground,
       ...shorthands.borderColor('transparent'),
     },
   },
   transparent: {
-    backgroundColor: 'transparent',
+    backgroundColor: tokens.colorTransparentBackground,
     ...shorthands.borderColor('transparent'),
 
     ':hover': {
-      backgroundColor: 'transparent',
+      backgroundColor: tokens.colorTransparentBackground,
       ...shorthands.borderColor('transparent'),
     },
 
     ':hover:active': {
-      backgroundColor: 'transparent',
+      backgroundColor: tokens.colorTransparentBackground,
       ...shorthands.borderColor('transparent'),
     },
   },
 });
 
 const useRootFocusStyles = makeStyles({
   base: createCustomFocusIndicatorStyle({
-    ...shorthands.borderColor('transparent'),
-    outlineColor: 'transparent',
+    ...shorthands.borderColor(tokens.colorTransparentStroke),
+    outlineColor: tokens.colorTransparentStroke,
     outlineWidth: tokens.strokeWidthThick,
     outlineStyle: 'solid',
     boxShadow: `
@@ -416,6 +416,17 @@ const useIconStyles = makeStyles({
     [iconSpacingVar]: tokens.spacingHorizontalSNudge,
   },
 
+  // Appearance variations
+  subtle: {
+    ':hover': {
+      color: tokens.colorNeutralForeground2BrandHover,
+    },
+
+    ':hover:active': {
+      color: tokens.colorNeutralForeground2BrandPressed,
+    },
+  },
+
   // Icon position variations
   before: {
     marginRight: `var(${iconSpacingVar})`,
@@ -467,6 +478,7 @@ export const useButtonStyles_unstable = (state: ButtonState): ButtonState => {
       buttonClassNames.icon,
       iconStyles.base,
       state.root.children !== undefined && state.root.children !== null && iconStyles[iconPosition],
+      appearance === 'subtle' && iconStyles.subtle,
       iconStyles[size],
       state.icon.className,
     );