fix: Fix menu item label style (#1556)

mondaycom · Oct 15, 2023 · babdb6d · babdb6d
1 parent 2ca70d5
commit babdb6d
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 25 deletions.
diff --git a/src/components/Label/__stories__/label.stories.mdx b/src/components/Label/__stories__/label.stories.mdx
@@ -109,25 +109,13 @@ A label indicates the status of an item.
 
 ### Clickable
 
-By applying the onClick prop, the label will become clickable and get all the relevant states (hover, focus, ...) and keyboard support
-
 <Canvas>
   <Story name="Clickable" parameters={{ chromatic: { pauseAnimationAtEnd: true } }}>
-    <div className="monday-storybook-label_group">
+    <div className="monday-storybook-label_group monday-storybook-label_states-gap">
       <Label text="New" onClick={NOOP} />
-      <Label text="New" kind={Label.kinds.LINE} onClick={NOOP} />
-    </div>
-    <div className="monday-storybook-label_group">
-      <Label text="New" color={Label.colors.NEGATIVE} onClick={NOOP} />
-      <Label text="New" color={Label.colors.NEGATIVE} kind={Label.kinds.LINE} onClick={NOOP} />
     </div>
-    <div className="monday-storybook-label_group">
-      <Label text="New" color={Label.colors.POSITIVE} onClick={NOOP} />
-      <Label text="New" color={Label.colors.POSITIVE} kind={Label.kinds.LINE} onClick={NOOP} />
-    </div>
-    <div className="monday-storybook-label_group">
-      <Label text="New" color={Label.colors.DARK} onClick={NOOP} />
-      <Label text="New" color={Label.colors.DARK} kind={Label.kinds.LINE} onClick={NOOP} />
+    <div className="monday-storybook-label_group monday-storybook-label_states-gap">
+      <Label text="New" kind={Label.kinds.LINE} onClick={NOOP} />
     </div>
   </Story>
 </Canvas>

diff --git a/src/components/Menu/MenuItem/MenuItem.module.scss b/src/components/Menu/MenuItem/MenuItem.module.scss
@@ -76,11 +76,3 @@
   flex-grow: 1;
   padding-block: 1px;
 }
-
-.label {
-  white-space: nowrap;
-  overflow: hidden;
-  text-overflow: ellipsis;
-  flex-grow: 0;
-  @include font-caption();
-}
diff --git a/src/components/Menu/MenuItem/MenuItem.tsx b/src/components/Menu/MenuItem/MenuItem.tsx
@@ -29,6 +29,7 @@ import { IconType } from "../../Icon/IconConstants";
 import { TAB_INDEX_FOCUS_WITH_JS_ONLY, TooltipPosition } from "./MenuItemConstants";
 import { CloseMenuOption } from "../Menu/MenuConstants";
 import styles from "./MenuItem.module.scss";
+import Label from "../../Label/Label";
 
 export interface MenuItemProps extends VibeComponentProps {
   title?: string;
@@ -314,7 +315,7 @@ const MenuItem: VibeComponent<MenuItemProps> & {
             {title}
           </div>
         </Tooltip>
-        {label && <div className={styles.label}>{label}</div>}
+        {label && <Label kind={Label.kinds.LINE} text={label} />}
         {renderSubMenuIconIfNeeded()}
         <div
           style={{ ...popoverStyles.popper, visibility: shouldShowSubMenu ? "visible" : "hidden" }}

diff --git a/src/components/Menu/MenuItem/__stories__/MenuItem.stories.js b/src/components/Menu/MenuItem/__stories__/MenuItem.stories.js
@@ -5,6 +5,7 @@ import Menu from "../../Menu/Menu";
 import { Activity, Bolt, Settings } from "../../../Icon/Icons";
 import Icon from "../../../Icon/Icon";
 import Tooltip from "../../../Tooltip/Tooltip";
+import DialogContentContainer from "../../../DialogContentContainer/DialogContentContainer";
 
 export const menuItemTemplate = args => (
   <Menu>
@@ -52,6 +53,12 @@ export const menuItemIconsWithColorsTemplate = args => {
   );
 };
 
+export const menuItemLabelTemplate = args => (
+  <Menu>
+    <MenuItem title="Menu item" label="New" />
+  </Menu>
+);
+
 export const menuItemOverflowTemplate = args => {
   return (
     <Menu>
@@ -89,3 +96,23 @@ export const menuItemTooltipTemplate = args => {
     </Menu>
   );
 };
+
+export const menuItemWithLabelDoTemplate = args => (
+  <DialogContentContainer>
+    <Menu>
+      <MenuItem title="Menu item 1" label="New" />
+      <MenuItem title="Menu item 2" />
+      <MenuItem title="Menu item 3" />
+    </Menu>
+  </DialogContentContainer>
+);
+
+export const menuItemWithLabelDontTemplate = args => (
+  <DialogContentContainer>
+    <Menu>
+      <MenuItem title="Menu Item 1" label="Long menu item label" />
+      <MenuItem title="Menu Item 2" />
+      <MenuItem title="Menu Item 3" />
+    </Menu>
+  </DialogContentContainer>
+);
diff --git a/src/components/Menu/MenuItem/__stories__/MenuItem.stories.mdx b/src/components/Menu/MenuItem/__stories__/MenuItem.stories.mdx
@@ -6,7 +6,10 @@ import {
   menuItemOverflowTemplate,
   menuItemStatesTemplate,
   menuItemTemplate,
-  menuItemTooltipTemplate
+  menuItemTooltipTemplate,
+  menuItemLabelTemplate,
+  menuItemWithLabelDoTemplate,
+  menuItemWithLabelDontTemplate
 } from "./MenuItem.stories";
 import MenuItem from "../MenuItem";
 
@@ -67,6 +70,14 @@ Use menu item for drawing one options that displayed inside a menu.
   <Story name="Icons with colors">{menuItemIconsWithColorsTemplate.bind({})}</Story>
 </Canvas>
 
+### Label
+
+<Canvas>
+  <Story name="Label" parameters={{ chromatic: { pauseAnimationAtEnd: true } }}>
+    {menuItemLabelTemplate.bind({})}
+  </Story>
+</Canvas>
+
 ### Overflow
 
 <Canvas>
@@ -78,3 +89,20 @@ Use menu item for drawing one options that displayed inside a menu.
 <Canvas>
   <Story name="Tooltip">{menuItemTooltipTemplate.bind({})}</Story>
 </Canvas>
+
+## Do's and Don’ts
+
+<ComponentRules
+  rules={[
+    {
+      positive: {
+        component: menuItemWithLabelDoTemplate(),
+        description: "Keep the label text in the menu item short and informative. Use 1 word only."
+      },
+      negative: {
+        component: menuItemWithLabelDontTemplate(),
+        description: "Dont use long text with more than 1 word, to ensure clarity and to avoid ellipsis."
+      }
+    }
+  ]}
+/>
diff --git a/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap b/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap
@@ -56,6 +56,49 @@ exports[`Snapshots renders correctly when selected 1`] = `
 </li>
 `;
 
+exports[`Snapshots renders correctly with a label 1`] = `
+<li
+  aria-current={false}
+  className="typography primary start singleLineEllipsis text text2Normal item"
+  data-testid="menu-item"
+  onClick={[Function]}
+  role="menuitem"
+  tabIndex={-1}
+>
+  <div
+    className="title"
+  >
+    my item
+  </div>
+  <span
+    className=""
+    data-testid="label"
+  >
+    <span
+      className="typography onInverted start singleLineEllipsis text text2Normal label kindLine colorPrimary withAnimation"
+      data-testid="text"
+    >
+      <span>
+        New
+      </span>
+      <span
+        className="legWrapper"
+      />
+    </span>
+  </span>
+  <div
+    style={
+      Object {
+        "left": "0",
+        "position": "absolute",
+        "top": "0",
+        "visibility": "hidden",
+      }
+    }
+  />
+</li>
+`;
+
 exports[`Snapshots renders correctly with custom class name 1`] = `
 <li
   aria-current={false}

diff --git a/src/components/Menu/MenuItem/__tests__/menuItem-snapshots.jest.js b/src/components/Menu/MenuItem/__tests__/menuItem-snapshots.jest.js
@@ -33,6 +33,11 @@ describe("Snapshots", () => {
     const tree = renderer.create(<MenuItem title="my item" selected={true} />).toJSON();
     expect(tree).toMatchSnapshot();
   });
+
+  it("renders correctly with a label", () => {
+    const tree = renderer.create(<MenuItem title="my item" label="New" />).toJSON();
+    expect(tree).toMatchSnapshot();
+  });
 });
 
 jest.useFakeTimers();