fix: Wrapping long text in Button components (#24682)

* fix: Wrapping long text in Button components. * Adding change file. * Adding vr tests and replacing min-height in buttons with padding+border+line-height calculation. * Removing uneeded text-overflow style. * Adding long text to long text vr stories and replacing 'truncate' with 'wrap' in storybook stories. * Adding long text to long text vr stories. Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>
microsoft · Sep 7, 2022 · 28f5559 · 28f5559
1 parent 22de26f
commit 28f5559
Show file tree

Hide file tree

Showing 8 changed files with 269 additions and 88 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,6 +2,7 @@ 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 { makeStyles } from '@griffel/react';
 
 const steps = new Screener.Steps()
   .snapshot('default', { cropTo: '.testWrapper' })
@@ -13,6 +14,12 @@ const steps = new Screener.Steps()
 
 const buttonId = 'button-id';
 
+const useStyles = makeStyles({
+  longText: {
+    width: '280px',
+  },
+});
+
 storiesOf('Button Converged', module)
   .addDecorator(story => <Screener steps={steps}>{story()}</Screener>)
   .addStory('Default', () => <Button id={buttonId}>Hello, world</Button>, {
@@ -127,6 +134,30 @@ storiesOf('Button Converged', module)
       Hello, world
     </Button>
   ))
+  .addStory('Size small - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <Button id={buttonId} className={styles.longText} icon="X" 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">
+        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">
+        Long text wraps after it hits the max width of the component
+      </Button>
+    );
+  })
   .addStory(
     'With icon before content',
     () => (
@@ -257,7 +288,48 @@ storiesOf('CompoundButton Converged', module)
       Hello, world
     </CompoundButton>
   ))
-
+  .addStory('Size small - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <CompoundButton
+        id={buttonId}
+        className={styles.longText}
+        secondaryContent="This is some secondary text"
+        icon="X"
+        size="small"
+      >
+        Long text wraps after it hits the max width of the component
+      </CompoundButton>
+    );
+  })
+  .addStory('Size medium - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <CompoundButton
+        id={buttonId}
+        className={styles.longText}
+        secondaryContent="This is some secondary text"
+        icon="X"
+        size="medium"
+      >
+        Long text wraps after it hits the max width of the component
+      </CompoundButton>
+    );
+  })
+  .addStory('Size large - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <CompoundButton
+        id={buttonId}
+        className={styles.longText}
+        secondaryContent="This is some secondary text"
+        icon="X"
+        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,
@@ -332,6 +404,30 @@ storiesOf('ToggleButton Converged', module)
       Hello, world
     </ToggleButton>
   ))
+  .addStory('Size small - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <ToggleButton id={buttonId} className={styles.longText} icon="X" 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">
+        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">
+        Long text wraps after it hits the max width of the component
+      </ToggleButton>
+    );
+  })
   .addStory('With icon before content', () => (
     <ToggleButton id={buttonId} icon="X">
       Hello, world
@@ -428,6 +524,30 @@ storiesOf('MenuButton Converged', module)
       Hello, world
     </MenuButton>
   ))
+  .addStory('Size small - with long text wrapping', () => {
+    const styles = useStyles();
+    return (
+      <MenuButton id={buttonId} className={styles.longText} icon="X" 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">
+        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">
+        Long text wraps after it hits the max width of the component
+      </MenuButton>
+    );
+  })
   .addStory('With icon', () => (
     <MenuButton id={buttonId} icon="X">
       Hello, world

diff --git a/change/@fluentui-react-button-2d7b0be2-d4e1-469e-8bf8-032018a98cfb.json b/change/@fluentui-react-button-2d7b0be2-d4e1-469e-8bf8-032018a98cfb.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: Wrapping long text in Button components.",
+  "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
@@ -12,6 +12,9 @@ export const buttonClassNames: SlotClassNames<ButtonSlots> = {
 
 const iconSpacingVar = '--fui-Button__icon--spacing';
 
+const buttonSpacingSmall = '3px';
+const buttonSpacingMedium = '5px';
+
 const useRootStyles = makeStyles({
   // Base styles
   base: {
@@ -23,8 +26,6 @@ const useRootStyles = makeStyles({
     ...shorthands.margin(0),
 
     ...shorthands.overflow('hidden'),
-    textOverflow: 'ellipsis',
-    whiteSpace: 'nowrap',
 
     backgroundColor: tokens.colorNeutralBackground1,
     color: tokens.colorNeutralForeground1,
@@ -168,21 +169,19 @@ const useRootStyles = makeStyles({
 
   // Size variations
   small: {
-    ...shorthands.padding(tokens.spacingVerticalNone, tokens.spacingHorizontalS),
+    ...shorthands.padding(buttonSpacingSmall, tokens.spacingHorizontalS),
 
-    height: '24px',
     minWidth: '64px',
 
-    ...shorthands.borderRadius(tokens.borderRadiusMedium),
+    ...shorthands.borderRadius(buttonSpacingSmall),
 
     fontSize: tokens.fontSizeBase200,
     fontWeight: tokens.fontWeightRegular,
     lineHeight: tokens.lineHeightBase200,
   },
   medium: {
-    ...shorthands.padding(tokens.spacingVerticalNone, tokens.spacingHorizontalM),
+    ...shorthands.padding(buttonSpacingMedium, tokens.spacingHorizontalM),
 
-    height: '32px',
     minWidth: '96px',
 
     ...shorthands.borderRadius(tokens.borderRadiusMedium),
@@ -192,9 +191,8 @@ const useRootStyles = makeStyles({
     lineHeight: tokens.lineHeightBase300,
   },
   large: {
-    ...shorthands.padding(tokens.spacingVerticalNone, tokens.spacingHorizontalL),
+    ...shorthands.padding(tokens.spacingVerticalS, tokens.spacingHorizontalL),
 
-    height: '40px',
     minWidth: '96px',
 
     ...shorthands.borderRadius(tokens.borderRadiusMedium),

diff --git a/packages/react-components/react-button/src/stories/Button/ButtonWithLongText.stories.tsx b/packages/react-components/react-button/src/stories/Button/ButtonWithLongText.stories.tsx
@@ -1,16 +1,27 @@
 import * as React from 'react';
-import { Button } from '@fluentui/react-components';
+import { makeStyles, Button } from '@fluentui/react-components';
+
+const useStyles = makeStyles({
+  longText: {
+    width: '280px',
+  },
+});
+
+export const WithLongText = () => {
+  const styles = useStyles();
+
+  return (
+    <>
+      <Button>Short text</Button>
+      <Button className={styles.longText}>Long text wraps after it hits the max width of the component</Button>
+    </>
+  );
+};
 
-export const WithLongText = () => (
-  <>
-    <Button>Short text</Button>
-    <Button>Long text truncates after it hits the max width of the component</Button>
-  </>
-);
 WithLongText.parameters = {
   docs: {
     description: {
-      story: 'Text truncates after it hits the max width of the component.',
+      story: 'Text wraps after it hits the max width of the component.',
     },
   },
 };
diff --git a/...components/react-button/src/stories/CompoundButton/CompoundButtonWithLongText.stories.tsx b/...components/react-button/src/stories/CompoundButton/CompoundButtonWithLongText.stories.tsx
@@ -16,15 +16,15 @@ export const WithLongText = () => {
         Short text
       </CompoundButton>
       <CompoundButton className={styles.maxWidth} secondaryContent="Secondary content">
-        Long text truncates after it hits the max width of the component
+        Long text wraps after it hits the max width of the component
       </CompoundButton>
     </>
   );
 };
 WithLongText.parameters = {
   docs: {
     description: {
-      story: 'Text truncates after it hits the max width of the component.',
+      story: 'Text wraps after it hits the max width of the component.',
     },
   },
 };
diff --git a/...s/react-components/react-button/src/stories/MenuButton/MenuButtonWithLongText.stories.tsx b/...s/react-components/react-button/src/stories/MenuButton/MenuButtonWithLongText.stories.tsx
@@ -1,39 +1,52 @@
 import * as React from 'react';
-import { Menu, MenuButton, MenuItem, MenuList, MenuPopover, MenuTrigger } from '@fluentui/react-components';
+import { makeStyles, Menu, MenuButton, MenuItem, MenuList, MenuPopover, MenuTrigger } from '@fluentui/react-components';
 
-export const WithLongText = () => (
-  <>
-    <Menu>
-      <MenuTrigger>
-        <MenuButton>Short text</MenuButton>
-      </MenuTrigger>
+const useStyles = makeStyles({
+  longText: {
+    width: '280px',
+  },
+});
+
+export const WithLongText = () => {
+  const styles = useStyles();
+
+  return (
+    <>
+      <Menu>
+        <MenuTrigger>
+          <MenuButton>Short text</MenuButton>
+        </MenuTrigger>
 
-      <MenuPopover>
-        <MenuList>
-          <MenuItem>Item a</MenuItem>
-          <MenuItem>Item b</MenuItem>
-        </MenuList>
-      </MenuPopover>
-    </Menu>
+        <MenuPopover>
+          <MenuList>
+            <MenuItem>Item a</MenuItem>
+            <MenuItem>Item b</MenuItem>
+          </MenuList>
+        </MenuPopover>
+      </Menu>
 
-    <Menu>
-      <MenuTrigger>
-        <MenuButton>Long text truncates after it hits the max width of the component</MenuButton>
-      </MenuTrigger>
+      <Menu>
+        <MenuTrigger>
+          <MenuButton className={styles.longText}>
+            Long text wraps after it hits the max width of the component
+          </MenuButton>
+        </MenuTrigger>
+
+        <MenuPopover>
+          <MenuList>
+            <MenuItem>Item a</MenuItem>
+            <MenuItem>Item b</MenuItem>
+          </MenuList>
+        </MenuPopover>
+      </Menu>
+    </>
+  );
+};
 
-      <MenuPopover>
-        <MenuList>
-          <MenuItem>Item a</MenuItem>
-          <MenuItem>Item b</MenuItem>
-        </MenuList>
-      </MenuPopover>
-    </Menu>
-  </>
-);
 WithLongText.parameters = {
   docs: {
     description: {
-      story: 'Text truncates after it hits the max width of the component.',
+      story: 'Text wraps after it hits the max width of the component.',
     },
   },
 };