fix(controls): Add forwardRef to Dropdown and Checkbox (#1381)

* fix(controls): Add forwardRef to Dropdown and Checkbox * chore: pr comments * chore: pr comments Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
box · May 14, 2021 · 79fbd40 · 79fbd40
1 parent 87e36b3
commit 79fbd40
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 52 deletions.
diff --git a/src/lib/types/react-augment.d.ts b/src/lib/types/react-augment.d.ts
@@ -0,0 +1,7 @@
+import React from 'react';
+
+declare module 'react' {
+    function forwardRef<T, P = {}>(
+        render: (props: P, ref: React.ForwardedRef<T>) => React.ReactElement | null,
+    ): (props: P & RefAttributes<T>) => React.ReactElement | null;
+}
diff --git a/src/lib/viewers/controls/settings/SettingsCheckboxItem.tsx b/src/lib/viewers/controls/settings/SettingsCheckboxItem.tsx
@@ -8,7 +8,10 @@ export type Props = {
     onChange: (isChecked: boolean) => void;
 };
 
-export default function SettingsCheckboxItem({ isChecked, label, onChange }: Props): JSX.Element {
+export type Ref = HTMLInputElement;
+
+function SettingsCheckboxItem(props: Props, ref: React.Ref<Ref>): JSX.Element {
+    const { isChecked, label, onChange } = props;
     const { current: id } = React.useRef(uniqueId('bp-SettingsCheckboxItem_'));
 
     const handleChange = (event: React.ChangeEvent<HTMLInputElement>): void => {
@@ -18,6 +21,7 @@ export default function SettingsCheckboxItem({ isChecked, label, onChange }: Pro
     return (
         <div className="bp-SettingsCheckboxItem">
             <input
+                ref={ref}
                 checked={isChecked}
                 className="bp-SettingsCheckboxItem-input"
                 id={id}
@@ -30,3 +34,5 @@ export default function SettingsCheckboxItem({ isChecked, label, onChange }: Pro
         </div>
     );
 }
+
+export default React.forwardRef(SettingsCheckboxItem);
diff --git a/src/lib/viewers/controls/settings/SettingsDropdown.scss b/src/lib/viewers/controls/settings/SettingsDropdown.scss
@@ -3,13 +3,19 @@
 $bp-SettingsDropdown-spacing: 5px;
 
 .bp-SettingsDropdown {
-    position: relative;
     display: flex;
     flex: 1 1 auto;
     flex-direction: column;
     color: $bdl-gray-62;
 }
 
+.bp-SettingsDropdown-content {
+    position: relative;
+    display: flex;
+    flex: 1 1 auto;
+    flex-direction: column;
+}
+
 .bp-SettingsDropdown-flyout {
     top: auto;
     right: 0;

diff --git a/src/lib/viewers/controls/settings/SettingsDropdown.tsx b/src/lib/viewers/controls/settings/SettingsDropdown.tsx
@@ -22,17 +22,13 @@ export type Props<V extends Value = string> = {
     value?: V;
 };
 
-export default function SettingsDropdown<V extends Value = string>({
-    className,
-    label,
-    listItems,
-    onSelect,
-    value,
-}: Props<V>): JSX.Element {
+export type Ref = HTMLButtonElement | null;
+
+function SettingsDropdown<V extends Value = string>(props: Props<V>, ref: React.Ref<Ref>): JSX.Element {
+    const { className, label, listItems, onSelect, value } = props;
     const { current: id } = React.useRef(uniqueId('bp-SettingsDropdown_'));
     const buttonElRef = React.useRef<HTMLButtonElement | null>(null);
     const dropdownElRef = React.useRef<HTMLDivElement | null>(null);
-    const listElRef = React.useRef<HTMLDivElement | null>(null);
     const [isOpen, setIsOpen] = React.useState(false);
 
     const handleKeyDown = (event: React.KeyboardEvent): void => {
@@ -74,52 +70,58 @@ export default function SettingsDropdown<V extends Value = string>({
 
     useClickOutside(dropdownElRef, () => setIsOpen(false));
 
+    // Customize the forwarded ref to combine usage with the ref internal to this component
+    React.useImperativeHandle(ref, () => buttonElRef.current, []);
+
     return (
-        <div ref={dropdownElRef} className={classNames('bp-SettingsDropdown', className)}>
+        <div className={classNames('bp-SettingsDropdown', className)}>
             <div className="bp-SettingsDropdown-label" id={`${id}-label`}>
                 {label}
             </div>
-            <button
-                ref={buttonElRef}
-                aria-expanded={isOpen}
-                aria-haspopup="listbox"
-                aria-labelledby={`${id}-label ${id}-button`}
-                className={classNames('bp-SettingsDropdown-button', { 'bp-is-open': isOpen })}
-                id={`${id}-button`}
-                onClick={(): void => setIsOpen(!isOpen)}
-                type="button"
-            >
-                {value}
-            </button>
-            <SettingsFlyout className="bp-SettingsDropdown-flyout" isOpen={isOpen}>
-                <SettingsList
-                    ref={listElRef}
-                    aria-labelledby={`${id}-label`}
-                    className="bp-SettingsDropdown-list"
-                    isActive
-                    onKeyDown={handleKeyDown}
-                    role="listbox"
-                    tabIndex={-1}
+            <div ref={dropdownElRef} className="bp-SettingsDropdown-content">
+                <button
+                    ref={buttonElRef}
+                    aria-expanded={isOpen}
+                    aria-haspopup="listbox"
+                    aria-labelledby={`${id}-label ${id}-button`}
+                    className={classNames('bp-SettingsDropdown-button', { 'bp-is-open': isOpen })}
+                    id={`${id}-button`}
+                    onClick={(): void => setIsOpen(!isOpen)}
+                    type="button"
                 >
-                    {listItems.map(({ label: itemLabel, value: itemValue }) => {
-                        const itemValueString = itemValue.toString();
-                        return (
-                            <div
-                                key={itemValueString}
-                                aria-selected={value === itemValue}
-                                className="bp-SettingsDropdown-listitem"
-                                id={itemValueString}
-                                onClick={createClickHandler(itemValue)}
-                                onKeyDown={createKeyDownHandler(itemValue)}
-                                role="option"
-                                tabIndex={0}
-                            >
-                                {itemLabel}
-                            </div>
-                        );
-                    })}
-                </SettingsList>
-            </SettingsFlyout>
+                    {value}
+                </button>
+                <SettingsFlyout className="bp-SettingsDropdown-flyout" isOpen={isOpen}>
+                    <SettingsList
+                        aria-labelledby={`${id}-label`}
+                        className="bp-SettingsDropdown-list"
+                        isActive
+                        onKeyDown={handleKeyDown}
+                        role="listbox"
+                        tabIndex={-1}
+                    >
+                        {listItems.map(({ label: itemLabel, value: itemValue }) => {
+                            const itemValueString = itemValue.toString();
+                            return (
+                                <div
+                                    key={itemValueString}
+                                    aria-selected={value === itemValue}
+                                    className="bp-SettingsDropdown-listitem"
+                                    id={itemValueString}
+                                    onClick={createClickHandler(itemValue)}
+                                    onKeyDown={createKeyDownHandler(itemValue)}
+                                    role="option"
+                                    tabIndex={0}
+                                >
+                                    {itemLabel}
+                                </div>
+                            );
+                        })}
+                    </SettingsList>
+                </SettingsFlyout>
+            </div>
         </div>
     );
 }
+
+export default React.forwardRef(SettingsDropdown);
diff --git a/src/lib/viewers/controls/settings/__tests__/SettingsDropdown-test.tsx b/src/lib/viewers/controls/settings/__tests__/SettingsDropdown-test.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import { act } from 'react-dom/test-utils';
 import { mount, ReactWrapper } from 'enzyme';
-import SettingsDropdown, { Props } from '../SettingsDropdown';
+import SettingsDropdown, { Props, Ref as SettingsDropdownRef } from '../SettingsDropdown';
 import SettingsFlyout from '../SettingsFlyout';
 import SettingsList from '../SettingsList';
 
@@ -186,6 +186,27 @@ describe('SettingsDropdown', () => {
         });
     });
 
+    describe('ref', () => {
+        const TestComponent = (): JSX.Element => {
+            const ref = React.useRef<SettingsDropdownRef | null>(null);
+
+            React.useEffect(() => {
+                if (ref.current) {
+                    ref.current.focus();
+                }
+            }, []);
+            return <SettingsDropdown ref={ref} {...getDefaults()} />;
+        };
+
+        test('should be able to focus on the dropdown button', () => {
+            const wrapper = mount(<TestComponent />, {
+                attachTo: getHostNode(),
+            });
+
+            expect(wrapper.find('.bp-SettingsDropdown-button').getDOMNode()).toHaveFocus();
+        });
+    });
+
     describe('render', () => {
         test('should return a valid wrapper', () => {
             const wrapper = getWrapper();