feat(reply): Address comments

box · May 14, 2020 · df80773 · df80773
1 parent eb20b7f
commit df80773
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 40 deletions.
diff --git a/src/components/Popups/ReplyField/ItemRow.scss b/src/components/Popups/ReplyField/ItemRow.scss
@@ -1,6 +1,6 @@
 @import '~box-ui-elements/es/styles/variables';
 
-.ba-DefaultItemRow-email {
+.ba-ItemRow-email {
     color: $bdl-gray-62;
     font-size: 11px;
     line-height: 13px;

diff --git a/src/components/Popups/ReplyField/ItemRow.tsx b/src/components/Popups/ReplyField/ItemRow.tsx
@@ -9,23 +9,23 @@ export type Props = {
     name?: string;
 };
 
-const DefaultItemRow = ({ item, ...rest }: Props): JSX.Element | null => {
+const ItemRow = ({ item, ...rest }: Props): JSX.Element | null => {
     if (!item || !item.name) {
         return null;
     }
 
     return (
         <DatalistItem {...rest}>
-            <div className="ba-DefaultItemRow-name" data-testid="ba-DefaultItemRow-name">
+            <div className="ba-ItemRow-name" data-testid="ba-ItemRow-name">
                 {item.name}
             </div>
             {'email' in item && (
-                <div className="ba-DefaultItemRow-email" data-testid="ba-DefaultItemRow-email">
+                <div className="ba-ItemRow-email" data-testid="ba-ItemRow-email">
                     {item.email}
                 </div>
             )}
         </DatalistItem>
     );
 };
 
-export default DefaultItemRow;
+export default ItemRow;
diff --git a/src/components/Popups/ReplyField/ReplyField.tsx b/src/components/Popups/ReplyField/ReplyField.tsx
@@ -70,8 +70,6 @@ export default class ReplyField extends React.Component<Props, State> {
 
     componentDidMount(): void {
         this.focusEditor();
-        // restore PopupList if it was active
-        this.updatePopupReference();
     }
 
     componentWillUnmount(): void {
@@ -98,7 +96,7 @@ export default class ReplyField extends React.Component<Props, State> {
             }
 
             const isNameMatch = fuzzySearch(trimmedQuery, item.name, 0);
-            const isEmailMatch = 'email' in item && fuzzySearch(trimmedQuery, item?.email, 0);
+            const isEmailMatch = 'email' in item && fuzzySearch(trimmedQuery, item.email, 0);
 
             return isNameMatch || isEmailMatch;
         });
@@ -195,10 +193,7 @@ export default class ReplyField extends React.Component<Props, State> {
         return 'handled';
     };
 
-    handleArrowHelper = (
-        event: React.KeyboardEvent,
-        getNextIndex: (activeItemIndex: number, itemsLength: number) => number,
-    ): void => {
+    handleArrow = (event: React.KeyboardEvent): void => {
         const { activeItemIndex, editorState, popupReference } = this.state;
         const collaborators = this.getCollaborators(getActiveMentionForEditorState(editorState));
 
@@ -207,19 +202,17 @@ export default class ReplyField extends React.Component<Props, State> {
         }
 
         this.stopDefaultEvent(event);
-        const nextIndex = getNextIndex(activeItemIndex, collaborators.length);
-        this.setPopupListActiveItem(nextIndex);
-    };
 
-    handleDownArrow = (event: React.KeyboardEvent): void =>
-        this.handleArrowHelper(event, (activeItemIndex: number, itemsLength: number) =>
-            activeItemIndex === itemsLength - 1 ? 0 : activeItemIndex + 1,
-        );
+        let nextIndex = 0;
 
-    handleUpArrow = (event: React.KeyboardEvent): void =>
-        this.handleArrowHelper(event, (activeItemIndex: number, itemsLength: number) =>
-            activeItemIndex === 0 ? itemsLength - 1 : activeItemIndex - 1,
-        );
+        if (event.key === 'ArrowDown') {
+            nextIndex = activeItemIndex === collaborators.length - 1 ? 0 : activeItemIndex + 1;
+        } else if (event.key === 'ArrowUp') {
+            nextIndex = activeItemIndex === 0 ? collaborators.length - 1 : activeItemIndex - 1;
+        }
+
+        this.setPopupListActiveItem(nextIndex);
+    };
 
     render(): JSX.Element {
         const { className, isDisabled, itemRowAs, placeholder, ...rest } = this.props;
@@ -233,8 +226,8 @@ export default class ReplyField extends React.Component<Props, State> {
                     editorState={editorState}
                     handleReturn={this.handleReturn}
                     onChange={this.handleChange}
-                    onDownArrow={this.handleDownArrow}
-                    onUpArrow={this.handleUpArrow}
+                    onDownArrow={this.handleArrow}
+                    onUpArrow={this.handleArrow}
                     placeholder={placeholder}
                     readOnly={isDisabled}
                     stripPastedStyles

diff --git a/src/components/Popups/ReplyField/__tests__/ItemList-test.tsx b/src/components/Popups/ReplyField/__tests__/ItemList-test.tsx
@@ -17,14 +17,14 @@ describe('components/Popups/ReplyField/ItemList', () => {
         test('should render elements with correct props', () => {
             const wrapper = getWrapper({ activeItemIndex: 1 });
 
-            const itmeList = wrapper.find('[data-testid="ba-ItemList"]');
-            const firstChild = itmeList.childAt(0);
-            const secondChild = itmeList.childAt(1);
+            const itemList = wrapper.find('[data-testid="ba-ItemList"]');
+            const firstChild = itemList.childAt(0);
+            const secondChild = itemList.childAt(1);
 
-            expect(itmeList.props()).toMatchObject({
+            expect(itemList.props()).toMatchObject({
                 role: 'listbox',
             });
-            expect(itmeList.children()).toHaveLength(2);
+            expect(itemList.children()).toHaveLength(2);
             expect(firstChild.prop('className')).toEqual('ba-ItemList-row');
             expect(firstChild.prop('isActive')).toEqual(false);
             expect(secondChild.prop('isActive')).toEqual(true);

diff --git a/src/components/Popups/ReplyField/__tests__/ItemRow-test.tsx b/src/components/Popups/ReplyField/__tests__/ItemRow-test.tsx
@@ -3,7 +3,7 @@ import { shallow, ShallowWrapper } from 'enzyme';
 import DatalistItem from 'box-ui-elements/es/components/datalist-item';
 import ItemRow, { Props } from '../ItemRow';
 
-describe('components/Popups/ReplyField/DefaultItemRow', () => {
+describe('components/Popups/ReplyField/ItemRow', () => {
     const defaults: Props = {
         id: 'testid',
         item: { email: '[email protected]', id: 'testid', name: 'testname', type: 'user' },
@@ -37,14 +37,14 @@ describe('components/Popups/ReplyField/DefaultItemRow', () => {
         test('should render item name and email', () => {
             const wrapper = getWrapper();
 
-            expect(wrapper.find('[data-testid="ba-DefaultItemRow-name"]').text()).toBe('testname');
-            expect(wrapper.find('[data-testid="ba-DefaultItemRow-email"]').text()).toBe('[email protected]');
+            expect(wrapper.find('[data-testid="ba-ItemRow-name"]').text()).toBe('testname');
+            expect(wrapper.find('[data-testid="ba-ItemRow-email"]').text()).toBe('[email protected]');
         });
 
         test('should not render email if item has no email', () => {
             const wrapper = getWrapper({ item: { id: 'testid', name: 'testname', type: 'group' } });
 
-            expect(wrapper.exists('[data-testid="ba-DefaultItemRow-email"]')).toBeFalsy();
+            expect(wrapper.exists('[data-testid="ba-ItemRow-email"]')).toBeFalsy();
         });
     });
 });
diff --git a/src/components/Popups/ReplyField/__tests__/MentionItem-test.tsx b/src/components/Popups/ReplyField/__tests__/MentionItem-test.tsx
@@ -3,7 +3,7 @@ import { ContentState } from 'draft-js';
 import { shallow, ShallowWrapper } from 'enzyme';
 import MentionItem, { Props } from '../MentionItem';
 
-describe('components/Popups/ReplyField/DefaultItemRow', () => {
+describe('components/Popups/ReplyField/MentionItem', () => {
     const defaults: Props = {
         children: <div />,
         contentState: ({

diff --git a/src/components/Popups/ReplyField/__tests__/ReplyField-test.tsx b/src/components/Popups/ReplyField/__tests__/ReplyField-test.tsx
@@ -299,6 +299,7 @@ describe('components/Popups/ReplyField', () => {
 
         beforeEach(() => {
             mockKeyboardEvent = ({
+                key: 'ArrowDown',
                 preventDefault: jest.fn(),
                 stopPropagation: jest.fn(),
             } as unknown) as React.KeyboardEvent<HTMLDivElement>;
@@ -330,21 +331,22 @@ describe('components/Popups/ReplyField', () => {
 
         test('should do nothing if collaborators length is 0', () => {
             getCollaboratorsSpy.mockReturnValueOnce([]);
-            instance.handleArrowHelper(mockKeyboardEvent, () => 0);
+            instance.handleArrow(mockKeyboardEvent);
 
             expect(stopDefaultEventSpy).not.toBeCalled();
             expect(setActiveItemSpy).not.toBeCalled();
         });
 
         test('should increase index if key is down', () => {
-            instance.handleDownArrow(mockKeyboardEvent);
+            instance.handleArrow(mockKeyboardEvent);
             expect(setActiveItemSpy).toBeCalledWith(1);
         });
 
         test('should decrease index if key is up', () => {
             wrapper.setState({ activeItemIndex: 1 });
+            mockKeyboardEvent.key = 'ArrowUp';
 
-            instance.handleUpArrow(mockKeyboardEvent);
+            instance.handleArrow(mockKeyboardEvent);
             expect(setActiveItemSpy).toBeCalledWith(0);
         });
     });

diff --git a/src/components/Popups/ReplyField/__tests__/ReplyFieldContainer-test.tsx b/src/components/Popups/ReplyField/__tests__/ReplyFieldContainer-test.tsx
@@ -6,7 +6,7 @@ import { createStore } from '../../../../store';
 
 jest.mock('../ReplyField');
 
-describe('components/Popups/ReplyFieldContainer', () => {
+describe('components/Popups/ReplyField/ReplyFieldContainer', () => {
     const store = createStore({
         creator: {
             cursor: 0,

diff --git a/src/components/Popups/ReplyField/__tests__/withMentionDecorator-test.tsx b/src/components/Popups/ReplyField/__tests__/withMentionDecorator-test.tsx
@@ -1,7 +1,7 @@
 import { EditorState, CompositeDecorator, ContentState } from 'draft-js';
 import withMentionDecorator, { mentionStrategy } from '../withMentionDecorator';
 
-describe('components/Popups/ReplyField', () => {
+describe('components/Popups/ReplyField/withMentionDecorator', () => {
     test('should set decorator', () => {
         const mockEditorState = EditorState.createEmpty();