Merge pull request #423 from dcos-labs/mp/feat/DCOS-60200-disabled-dr…

…opdown-item DCOS-60200: support for disabled dropdown menu items
d2iq · Oct 28, 2019 · 046ad8c · 046ad8c
2 parents 52511d4 + 38465ec
commit 046ad8c
Show file tree

Hide file tree

Showing 11 changed files with 132 additions and 6 deletions.
diff --git a/packages/dropdownMenu/README.md b/packages/dropdownMenu/README.md
@@ -12,6 +12,9 @@ If you're not using a button as your dropdown trigger, be sure whatever you use
 ## Icons and avatars in items
 Dropdown menu items can display an icon or avatar before or after the menu item text. By default, the avatar or icon is sized at `iconSizeXs`.
 
+## Disabled menu items
+TODO: Collaborate with the design team to decide on some guidelines for disabled items. For example: when should we render a disabled item instead of not showing the item in the dropdown at all?
+
 ## Do's and Dont's
 
 ### Do

diff --git a/packages/dropdownMenu/components/DropdownMenu.tsx b/packages/dropdownMenu/components/DropdownMenu.tsx
@@ -95,7 +95,8 @@ const DropdownMenu: React.SFC<DropdownMenuProps> = props => {
               index={i}
               key={child.key || `${sectionIndex}-${i}`}
               {...getItemProps({
-                item: child
+                item: child,
+                disabled: child.props.disabled
               })}
             >
               {child}

diff --git a/packages/dropdownMenu/components/DropdownMenuItem.tsx b/packages/dropdownMenu/components/DropdownMenuItem.tsx
@@ -9,6 +9,7 @@ export interface DropdownMenuItemProps {
    * The value that the item represents
    */
   value: string;
+  disabled?: boolean;
   children: React.ReactNode;
 }
 

diff --git a/packages/dropdownMenu/stories/Dropdown.stories.tsx b/packages/dropdownMenu/stories/Dropdown.stories.tsx
@@ -60,6 +60,27 @@ storiesOf("DropdownMenu", module)
       </DropdownSection>
     </DropdownMenu>
   ))
+  .add("with disabled action", () => (
+    <DropdownMenu trigger={<PrimaryDropdownButton>Menu</PrimaryDropdownButton>}>
+      <DropdownSection>
+        <DropdownMenuItem key="edit" value="edit">
+          Edit
+        </DropdownMenuItem>
+        <DropdownMenuItem key="scale" value="scale">
+          Scale
+        </DropdownMenuItem>
+        <DropdownMenuItem key="restart" value="restart">
+          Restart
+        </DropdownMenuItem>
+        <DropdownMenuItem key="stop" value="stop">
+          Stop
+        </DropdownMenuItem>
+        <DropdownMenuItem disabled={true} key="delete" value="delete">
+          Delete
+        </DropdownMenuItem>
+      </DropdownSection>
+    </DropdownMenu>
+  ))
   .add("initialIsOpen", () => (
     <DropdownMenu
       initialIsOpen={true}

diff --git a/packages/dropdownMenu/tests/DropdownMenu.test.tsx b/packages/dropdownMenu/tests/DropdownMenu.test.tsx
@@ -242,4 +242,47 @@ describe("Dropdown", () => {
     // what to expect
     expect(onSelectFn).toHaveBeenCalledWith("edit", expect.anything());
   });
+  it("calls onSelect prop with the selected value", () => {
+    const onSelectFn = jest.fn();
+    const component = mount(
+      <DropdownMenu
+        onSelect={onSelectFn}
+        trigger={<div id={triggerId}>Dropdown trigger</div>}
+      >
+        <DropdownSection>
+          <DropdownMenuItem disabled={true} key="edit" value="edit">
+            Edit
+          </DropdownMenuItem>
+          <DropdownMenuItem key="scale" value="scale">
+            Scale
+          </DropdownMenuItem>
+          <DropdownMenuItem key="restart" value="restart">
+            Restart
+          </DropdownMenuItem>
+          <DropdownMenuItem key="stop" value="stop">
+            Stop
+          </DropdownMenuItem>
+        </DropdownSection>
+      </DropdownMenu>
+    );
+    const trigger = component.find(`#${triggerId}`);
+
+    trigger.simulate("focus");
+    trigger.simulate("keyDown", {
+      key: " " // space bar
+    });
+    expect(onSelectFn).not.toHaveBeenCalled();
+    trigger.simulate("keyDown", {
+      key: "ArrowDown"
+    });
+    trigger.simulate("keyDown", {
+      key: "Enter"
+    });
+    trigger.simulate("blur");
+
+    // using `expect.anything()` because the second parameter is an object
+    // that comes from Downshift, and there's no good way to know exactly
+    // what to expect
+    expect(onSelectFn).not.toHaveBeenCalledWith("edit", expect.anything());
+  });
 });
diff --git a/packages/popover/components/PopoverListItem.tsx b/packages/popover/components/PopoverListItem.tsx
@@ -11,7 +11,8 @@ import {
   themeBgSelected,
   themeTextColorPrimary,
   themeTextColorPrimaryInverted,
-  themeError
+  themeError,
+  themeTextColorDisabled
 } from "../../design-tokens/build/js/designTokens";
 import getCSSVarValue from "../../utilities/components/getCSSVarValue";
 import { darken, pickReadableTextColor } from "../../shared/styles/color";
@@ -37,7 +38,6 @@ const PopoverListItem = (props: PopoverListItemProps) => {
     isSelected,
     index,
     listLength,
-    disabled,
     children,
     ...other
   } = props;
@@ -55,6 +55,10 @@ const PopoverListItem = (props: PopoverListItemProps) => {
   const menuListItemSelectedActive = css`
     background-color: ${darken(getCSSVarValue(themeBgSelected), 1)};
   `;
+  const menuListItemDisabled = css`
+    background-color: transparent;
+    color: ${themeTextColorDisabled};
+  `;
 
   const {
     itemGraphicStart,
@@ -135,6 +139,7 @@ const PopoverListItem = (props: PopoverListItemProps) => {
           [menuListItemActive]: isActive,
           [menuListItemSelected]: isSelected,
           [menuListItemSelectedActive]: isActive && isSelected,
+          [menuListItemDisabled]: other.disabled,
           [margin("top", "xs")]: index === 0,
           [margin("bottom", "xs")]: index === listLength - 1,
           [tintContent(themeError)]: appearance === "danger"

diff --git a/packages/popover/tests/__snapshots__/PopoverListItem.test.tsx.snap b/packages/popover/tests/__snapshots__/PopoverListItem.test.tsx.snap
@@ -179,13 +179,16 @@ exports[`PopoverListItem renders disabled 1`] = `
   padding-top: 8px;
   padding-bottom: 8px;
   display: table;
+  background-color: transparent;
+  color: var(--themeTextColorDisabled,#AEB0B4);
   margin-top: 8px;
   margin-bottom: 8px;
 }
 
 <div
   className="emotion-0"
   data-cy="PopoverListItem"
+  disabled={true}
 >
   <Flex
     align="center"

diff --git a/packages/typeahead/components/Typeahead.tsx b/packages/typeahead/components/Typeahead.tsx
@@ -9,6 +9,7 @@ import resizeEventManager from "../../utilities/resizeEventManager";
 export interface Item {
   value: string;
   label: React.ReactNode;
+  disabled?: boolean;
 }
 
 export interface TypeaheadProps {
@@ -135,7 +136,8 @@ class Typeahead extends React.PureComponent<TypeaheadProps, TypeaheadState> {
                                   index={index}
                                   {...getItemProps({
                                     key: item.value,
-                                    item
+                                    item,
+                                    disabled: item.disabled
                                   })}
                                 >
                                   {item.label}

diff --git a/packages/typeahead/stories/Typeahead.stories.tsx b/packages/typeahead/stories/Typeahead.stories.tsx
@@ -125,6 +125,23 @@ storiesOf("Forms/Typeahead", module)
       />
     </div>
   ))
+  .add("with a disabled item", () => (
+    <div className={storyWrapper}>
+      <Typeahead
+        items={[
+          ...items,
+          { label: "K8sphere", value: "K8sphere", disabled: true }
+        ]}
+        textField={
+          <TextInput
+            id="default"
+            inputLabel="With a disabled item"
+            placeholder="Placeholder"
+          />
+        }
+      />
+    </div>
+  ))
   .add("filter while typing", () => (
     <div className={storyWrapper}>
       <FilteredListTypeahead items={items} />

diff --git a/packages/typeahead/stories/helpers/itemMocks.tsx b/packages/typeahead/stories/helpers/itemMocks.tsx
@@ -26,7 +26,7 @@ const ComplexItem = props => (
 
 export const items = [
   { label: "Exosphere", value: "Exosphere" },
-  { label: "Thermosphere", value: "Thermosphere", disabled: true },
+  { label: "Thermosphere", value: "Thermosphere" },
   { label: "Mesosphere", value: "Mesosphere" },
   { label: "Stratosphere", value: "Stratosphere" },
   { label: "Ozone Layer", value: "Ozone Layer" },

diff --git a/packages/typeahead/tests/Typeahead.test.tsx b/packages/typeahead/tests/Typeahead.test.tsx
@@ -171,7 +171,7 @@ describe("Typeahead", () => {
     expect(onSelectFn).toHaveBeenCalledWith([items[0].value], items[0].value);
   });
 
-  it("sets the input value to the selected item's value on change", () => {
+  it("sets the input value to the selected item's value on select", () => {
     const onSelectFn = jest.fn();
     const component = mount(
       <Typeahead
@@ -198,6 +198,36 @@ describe("Typeahead", () => {
     expect(component.find("input").prop("value")).toBe(items[0].value);
   });
 
+  it("does not set the input value to the selected item's value on select", () => {
+    const onSelectFn = jest.fn();
+    const component = mount(
+      <Typeahead
+        items={[
+          { label: "K8sphere", value: "K8sphere", disabled: true },
+          ...items
+        ]}
+        onSelect={onSelectFn}
+        textField={
+          <TextInput
+            id="standard.input"
+            inputLabel="Standard"
+            placeholder="Placeholder"
+          />
+        }
+      />
+    );
+    expect(component.find("input").prop("value")).toBe("");
+    component.find("input").simulate("focus");
+    expect(onSelectFn).not.toHaveBeenCalled();
+    component.find("input").simulate("keyDown", {
+      key: "ArrowDown"
+    });
+    component.find("input").simulate("keyDown", {
+      key: "Enter"
+    });
+    expect(component.find("input").prop("value")).not.toBe(items[0].value);
+  });
+
   it("does not hide the dropdown when selecting an item if multiSelect is true", () => {
     const onSelectFn = jest.fn();
     const component = mount(