Fix disabled SelectNext.Options which allow top be interacted

Author: acelaya

src/components/input/SelectNext.tsx

@@ -28,10 +28,25 @@ export type SelectOptionStatus = {
 export type SelectOptionProps<T> = {
   value: T;
   disabled?: boolean;
-  children: (status: SelectOptionStatus) => ComponentChildren;
+  children:
+    | ComponentChildren
+    | ((status: SelectOptionStatus) => ComponentChildren);
   classes?: string | string[];
 };
 
+function optionChildren(
+  children:
+    | ComponentChildren
+    | ((status: SelectOptionStatus) => ComponentChildren),
+  status: SelectOptionStatus,
+): ComponentChildren {
+  if (typeof children === 'function') {
+    return children(status);
+  }
+
+  return children;
+}
+
 function SelectOption<T>({
   value,
   children,
@@ -49,14 +64,18 @@ function SelectOption<T>({
   return (
     <li
       className={classnames(
-        'w-full ring-inset focus:ring outline-none rounded-none cursor-pointer',
+        'w-full ring-inset focus:ring outline-none rounded-none',
         'border-t first:border-t-0 transition-colors whitespace-nowrap',
-        { 'hover:bg-grey-1': !disabled },
+        { 'cursor-pointer hover:bg-grey-1': !disabled },
         classes,
       )}
-      onClick={() => selectValue(value)}
+      onClick={() => {
+        if (!disabled) {
+          selectValue(value);
+        }
+      }}
       onKeyPress={e => {
-        if (e.code === 'Enter' || e.code === 'Space') {
+        if (!disabled && ['Enter', 'Space'].includes(e.code)) {
           e.preventDefault();
           selectValue(value);
         }
@@ -73,7 +92,7 @@ function SelectOption<T>({
           'border-l-brand font-medium': selected,
         })}
       >
-        {children({ selected, disabled })}
+        {optionChildren(children, { selected, disabled })}
       </div>
     </li>
   );

src/components/input/test/SelectNext-test.js

@@ -81,6 +81,16 @@ describe('SelectNext', () => {
     assert.calledWith(onChange.lastCall, items[0]);
   });
 
+  it('does not change selected value when a disabled option is clicked', () => {
+    const onChange = sinon.stub();
+    const wrapper = createComponent({ onChange });
+    const clickDisabledOption = () =>
+      wrapper.find(`[data-testid="option-4"]`).simulate('click');
+
+    clickDisabledOption();
+    assert.notCalled(onChange);
+  });
+
   ['Enter', 'Space'].forEach(code => {
     it(`changes selected value when ${code} is pressed in option`, () => {
       const onChange = sinon.stub();
@@ -101,6 +111,20 @@ describe('SelectNext', () => {
       pressKeyInOption(1);
       assert.calledWith(onChange.lastCall, items[0]);
     });
+
+    it(`does not change selected value when ${code} is pressed in a disabled option`, () => {
+      const onChange = sinon.stub();
+      const wrapper = createComponent({ onChange });
+      const pressKeyInDisabledOption = () =>
+        wrapper
+          .find(`[data-testid="option-4"]`)
+          .getDOMNode()
+          .closest('[role="option"]')
+          .dispatchEvent(new KeyboardEvent('keypress', { code }));
+
+      pressKeyInDisabledOption();
+      assert.notCalled(onChange);
+    });
   });
 
   it('marks the right item as selected', () => {

src/pattern-library/components/patterns/prototype/SelectNextPage.tsx

@@ -55,19 +55,17 @@ function SelectExample({
     >
       {items.map(item => (
         <SelectNext.Option value={item} key={item.id}>
-          {() =>
-            textOnly ? (
-              <>{item.name}</>
-            ) : (
-              <>
-                {item.name}
-                <div className="grow" />
-                <div className="rounded px-2 ml-2 bg-grey-7 text-white">
-                  {item.id}
-                </div>
-              </>
-            )
-          }
+          {textOnly ? (
+            item.name
+          ) : (
+            <>
+              {item.name}
+              <div className="grow" />
+              <div className="rounded px-2 ml-2 bg-grey-7 text-white">
+                {item.id}
+              </div>
+            </>
+          )}
         </SelectNext.Option>
       ))}
     </SelectNext>
@@ -117,19 +115,13 @@ function InputGroupSelectExample({ classes }: { classes?: string }) {
       >
         {defaultItems.map(item => (
           <SelectNext.Option value={item} key={item.id}>
-            {() => (
-              <>
-                {item.name}
-                <div className="grow" />
-                <div
-                  className={classnames(
-                    'rounded px-2 ml-2 text-white bg-grey-7',
-                  )}
-                >
-                  {item.id}
-                </div>
-              </>
-            )}
+            {item.name}
+            <div className="grow" />
+            <div
+              className={classnames('rounded px-2 ml-2 text-white bg-grey-7')}
+            >
+              {item.id}
+            </div>
           </SelectNext.Option>
         ))}
       </SelectNext>
@@ -328,6 +320,46 @@ export default function SelectNextPage() {
               </Library.InfoItem>
             </Library.Info>
           </Library.Example>
+          <p>
+            Every <code>SelectNext.Option</code> has its own set of props.
+          </p>
+          <Library.Example title="children">
+            <Library.Info>
+              <Library.InfoItem label="description">
+                Content of the option. You can pass a callback to receive the
+                option status (<code>disabled</code> and <code>selected</code>).
+              </Library.InfoItem>
+              <Library.InfoItem label="type">
+                <code>
+                  ComponentChildren | (({'{'} disabled, selected {'}'}) {'=>'}{' '}
+                  ComponentChildren)
+                </code>
+              </Library.InfoItem>
+            </Library.Info>
+          </Library.Example>
+          <Library.Example title="value">
+            <Library.Info>
+              <Library.InfoItem label="description">
+                The value to set when this option is selected.
+              </Library.InfoItem>
+              <Library.InfoItem label="type">
+                <code>T</code>
+              </Library.InfoItem>
+            </Library.Info>
+          </Library.Example>
+          <Library.Example title="disabled">
+            <Library.Info>
+              <Library.InfoItem label="description">
+                Whether the option is disabled or not.
+              </Library.InfoItem>
+              <Library.InfoItem label="type">
+                <code>boolean</code>
+              </Library.InfoItem>
+              <Library.InfoItem label="default">
+                <code>undefined</code>
+              </Library.InfoItem>
+            </Library.Info>
+          </Library.Example>
         </Library.Pattern>
 
         <Library.Pattern title="How to use it">