fix(mentions): Fix collabs list scrolling via keys (#532)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
box · Jun 29, 2020 · ed6cab6 · ed6cab6
1 parent ff797ff
commit ed6cab6
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 44 deletions.
diff --git a/src/components/ItemList/ItemList.tsx b/src/components/ItemList/ItemList.tsx
@@ -1,10 +1,12 @@
 import React from 'react';
 import noop from 'lodash/noop';
-import ItemRow from './ItemRow';
+import scrollIntoView from 'scroll-into-view-if-needed';
+import ItemRow, { ItemRowRef } from './ItemRow';
 import './ItemList.scss';
 
 export type Props<T extends { id: string }> = {
     activeItemIndex?: number;
+    autoScroll?: boolean;
     itemRowAs?: React.ElementType;
     items: T[];
     onActivate?: (index: number) => void;
@@ -13,31 +15,49 @@ export type Props<T extends { id: string }> = {
 
 const ItemList = <T extends { id: string }>({
     activeItemIndex = 0,
+    autoScroll = false,
     itemRowAs: ItemRowComponent = ItemRow,
     items,
     onActivate = noop,
     onSelect,
     ...rest
-}: Props<T>): JSX.Element => (
-    <ul className="ba-ItemList" data-testid="ba-ItemList" role="listbox" {...rest}>
-        {items.map((item, index) => (
-            <ItemRowComponent
-                key={item.id}
-                className="ba-ItemList-row"
-                isActive={index === activeItemIndex}
-                item={item}
-                onClick={(event: React.SyntheticEvent) => {
-                    onSelect(index, event);
-                }}
-                onMouseDown={(event: React.SyntheticEvent) => {
-                    event.preventDefault(); // preventDefault on mousedown so blur doesn't happen before click
-                }}
-                onMouseEnter={() => {
-                    onActivate(index);
-                }}
-            />
-        ))}
-    </ul>
-);
+}: Props<T>): JSX.Element => {
+    const activeRef = React.useCallback(
+        (activeEl: ItemRowRef) => {
+            if (!autoScroll || !activeEl) {
+                return;
+            }
+
+            scrollIntoView(activeEl, {
+                scrollMode: 'if-needed',
+                block: 'nearest',
+            });
+        },
+        [autoScroll],
+    );
+
+    return (
+        <ul className="ba-ItemList" data-testid="ba-ItemList" role="listbox" {...rest}>
+            {items.map((item, index) => (
+                <ItemRowComponent
+                    key={item.id}
+                    ref={index === activeItemIndex ? activeRef : null}
+                    className="ba-ItemList-row"
+                    isActive={index === activeItemIndex}
+                    item={item}
+                    onClick={(event: React.SyntheticEvent) => {
+                        onSelect(index, event);
+                    }}
+                    onMouseDown={(event: React.SyntheticEvent) => {
+                        event.preventDefault(); // preventDefault on mousedown so blur doesn't happen before click
+                    }}
+                    onMouseEnter={() => {
+                        onActivate(index);
+                    }}
+                />
+            ))}
+        </ul>
+    );
+};
 
 export default ItemList;
diff --git a/src/components/ItemList/ItemRow.tsx b/src/components/ItemList/ItemRow.tsx
@@ -12,14 +12,11 @@ export type Props = {
     onMouseEnter?: (event: React.SyntheticEvent) => void;
 };
 
-const ItemRow = ({
-    className,
-    isActive,
-    item: collaborator,
-    onClick,
-    onMouseDown,
-    onMouseEnter,
-}: Props): JSX.Element | null => {
+export type ItemRowRef = HTMLLIElement;
+
+const ItemRow = (props: Props, ref: React.Ref<ItemRowRef>): JSX.Element | null => {
+    const { className, isActive, item: collaborator, onClick, onMouseDown, onMouseEnter } = props;
+
     if (!collaborator || !collaborator.item || !collaborator.item.name) {
         return null;
     }
@@ -29,6 +26,7 @@ const ItemRow = ({
     return (
         // eslint-disable-next-line jsx-a11y/click-events-have-key-events
         <li
+            ref={ref}
             aria-selected={isActive}
             className={classNames(className, 'ba-ItemRow', { 'is-active': isActive })}
             data-testid="ba-ItemRow"
@@ -50,4 +48,4 @@ const ItemRow = ({
     );
 };
 
-export default ItemRow;
+export default React.forwardRef(ItemRow);
diff --git a/src/components/ItemList/__tests__/ItemList-test.tsx b/src/components/ItemList/__tests__/ItemList-test.tsx
@@ -1,8 +1,11 @@
 import React from 'react';
-import { shallow, ShallowWrapper } from 'enzyme';
+import scrollIntoView from 'scroll-into-view-if-needed';
+import { mount, ReactWrapper } from 'enzyme';
 import ItemList, { Props } from '../ItemList';
 import { Collaborator } from '../../../@types';
 
+jest.mock('scroll-into-view-if-needed', () => jest.fn());
+
 describe('components/ItemList/ItemList', () => {
     const defaults: Props<Collaborator> = {
         items: [
@@ -12,7 +15,7 @@ describe('components/ItemList/ItemList', () => {
         onSelect: jest.fn(),
     };
 
-    const getWrapper = (props = {}): ShallowWrapper => shallow(<ItemList {...defaults} {...props} />);
+    const getWrapper = (props = {}): ReactWrapper => mount(<ItemList {...defaults} {...props} />);
 
     describe('render()', () => {
         test('should render elements with correct props', () => {
@@ -41,13 +44,23 @@ describe('components/ItemList/ItemList', () => {
             const firstChild = wrapper.find('[data-testid="ba-ItemList"]').childAt(0);
 
             firstChild.simulate('click', mockEvent);
-            expect(defaults.onSelect).toBeCalledWith(0, mockEvent);
+            expect(defaults.onSelect).toBeCalledWith(0, expect.objectContaining(mockEvent));
 
             firstChild.simulate('mousedown', mockEvent);
             expect(mockEvent.preventDefault).toBeCalled();
 
             firstChild.simulate('mouseenter');
             expect(mockSetIndex).toBeCalledWith(0);
         });
+
+        test('should call scrollIntoView if autoScroll is true', () => {
+            const wrapper = getWrapper();
+
+            expect(scrollIntoView).not.toBeCalled();
+
+            wrapper.setProps({ autoScroll: true });
+
+            expect(scrollIntoView).toBeCalled();
+        });
     });
 });
diff --git a/src/components/Popups/PopupList.tsx b/src/components/Popups/PopupList.tsx
@@ -9,6 +9,7 @@ import './PopupList.scss';
 
 export type Props<T extends { id: string }> = {
     activeItemIndex?: number;
+    autoScroll?: boolean;
     items: T[];
     onActivate?: (index: number) => void;
     onSelect: (index: number, event: React.SyntheticEvent) => void;

diff --git a/src/components/ReplyField/ReplyField.tsx b/src/components/ReplyField/ReplyField.tsx
@@ -33,6 +33,7 @@ export type Props = {
 
 export type State = {
     activeItemIndex: number;
+    popupAutoScroll: boolean;
     popupReference: VirtualElement | null;
 };
 
@@ -43,7 +44,7 @@ export default class ReplyField extends React.Component<Props, State> {
         isDisabled: false,
     };
 
-    state: State = { activeItemIndex: 0, popupReference: null };
+    state: State = { activeItemIndex: 0, popupAutoScroll: false, popupReference: null };
 
     fetchCollaborators = debounce((editorState: EditorState): void => {
         const { fetchCollaborators } = this.props;
@@ -116,7 +117,8 @@ export default class ReplyField extends React.Component<Props, State> {
         event.stopPropagation();
     };
 
-    setPopupListActiveItem = (index: number): void => this.setState({ activeItemIndex: index });
+    setPopupListActiveItem = (index: number, autoScroll = false): void =>
+        this.setState({ activeItemIndex: index, popupAutoScroll: autoScroll });
 
     handleChange = (nextEditorState: EditorState): void => {
         const { onChange } = this.props;
@@ -170,7 +172,7 @@ export default class ReplyField extends React.Component<Props, State> {
             return;
         }
 
-        this.setPopupListActiveItem(activeItemIndex === 0 ? length - 1 : activeItemIndex - 1);
+        this.setPopupListActiveItem(activeItemIndex === 0 ? length - 1 : activeItemIndex - 1, true);
     };
 
     handleDownArrow = (event: React.KeyboardEvent): void => {
@@ -181,12 +183,12 @@ export default class ReplyField extends React.Component<Props, State> {
             return;
         }
 
-        this.setPopupListActiveItem(activeItemIndex === length - 1 ? 0 : activeItemIndex + 1);
+        this.setPopupListActiveItem(activeItemIndex === length - 1 ? 0 : activeItemIndex + 1, true);
     };
 
     render(): JSX.Element {
         const { className, collaborators, editorState, isDisabled, placeholder, ...rest } = this.props;
-        const { activeItemIndex, popupReference } = this.state;
+        const { activeItemIndex, popupAutoScroll, popupReference } = this.state;
 
         return (
             <div className={classnames(className, 'ba-ReplyField')}>
@@ -206,6 +208,7 @@ export default class ReplyField extends React.Component<Props, State> {
                 {popupReference && (
                     <PopupList
                         activeItemIndex={activeItemIndex}
+                        autoScroll={popupAutoScroll}
                         items={collaborators}
                         onActivate={this.setPopupListActiveItem}
                         onSelect={this.handleSelect}

diff --git a/src/components/ReplyField/__tests__/ReplyField-test.tsx b/src/components/ReplyField/__tests__/ReplyField-test.tsx
@@ -215,10 +215,13 @@ describe('components/Popups/ReplyField', () => {
     });
 
     describe('setPopupListActiveItem()', () => {
-        const wrapper = getWrapper();
-        wrapper.instance().setPopupListActiveItem(1);
+        test('should set state', () => {
+            const wrapper = getWrapper();
+            wrapper.instance().setPopupListActiveItem(1, true);
 
-        expect(wrapper.state('activeItemIndex')).toBe(1);
+            expect(wrapper.state('activeItemIndex')).toBe(1);
+            expect(wrapper.state('popupAutoScroll')).toBe(true);
+        });
     });
 
     describe('handleKeyDown', () => {
@@ -265,14 +268,14 @@ describe('components/Popups/ReplyField', () => {
 
         test('should increase index if key is down', () => {
             instance.handleDownArrow(mockKeyboardEvent);
-            expect(setActiveItemSpy).toBeCalledWith(1);
+            expect(setActiveItemSpy).toBeCalledWith(1, true);
         });
 
         test('should decrease index if key is up', () => {
             wrapper.setState({ activeItemIndex: 1 });
 
             instance.handleUpArrow(mockKeyboardEvent);
-            expect(setActiveItemSpy).toBeCalledWith(0);
+            expect(setActiveItemSpy).toBeCalledWith(0, true);
         });
     });