From f9fe3b97978dc5983a58d5146aa78dd488d9b891 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 31 Aug 2021 16:51:18 -0600 Subject: [PATCH 1/6] Sort contacts alphabetically Contacts are grouped together by letter, and the groups are listed alphabetically, but the contacts in each group are not sorted alphabetically themselves. Fixes #10318. --- .eslintrc.js | 7 ++- test/jest/index.js | 2 + test/jest/matchers.js | 35 +++++++++++ test/jest/setup.js | 1 + .../contact-list/contact-list.component.js | 55 +++++++++-------- .../app/contact-list/contact-list.test.js | 60 +++++++++++++++++++ 6 files changed, 133 insertions(+), 27 deletions(-) create mode 100644 test/jest/matchers.js create mode 100644 ui/components/app/contact-list/contact-list.test.js diff --git a/.eslintrc.js b/.eslintrc.js index b6094c794e6f..ae8b384bff5b 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -129,7 +129,12 @@ module.exports = { }, }, { - files: ['ui/**/*.test.js', 'ui/__mocks__/*.js', 'shared/**/*.test.js'], + files: [ + 'ui/**/*.test.js', + 'ui/__mocks__/*.js', + 'shared/**/*.test.js', + 'test/jest/*.js', + ], extends: ['@metamask/eslint-config-jest'], rules: { 'jest/no-restricted-matchers': 'off', diff --git a/test/jest/index.js b/test/jest/index.js index 098877489808..b68672d34541 100644 --- a/test/jest/index.js +++ b/test/jest/index.js @@ -1,3 +1,5 @@ +import './matchers'; + export { createSwapsMockStore } from './mock-store'; export { renderWithProvider } from './rendering'; export { setBackgroundConnection } from './background'; diff --git a/test/jest/matchers.js b/test/jest/matchers.js new file mode 100644 index 000000000000..660018b40ed0 --- /dev/null +++ b/test/jest/matchers.js @@ -0,0 +1,35 @@ +import { shallow, mount } from 'enzyme'; + +const SHALLOW_WRAPPER_CONSTRUCTOR = 'ShallowWrapper'; + +function isShallowWrapper(wrapper) { + return wrapper.constructor.name + ? wrapper.constructor.name === SHALLOW_WRAPPER_CONSTRUCTOR + : Boolean(`${wrapper.constructor}`.match(/^function ShallowWrapper\(/u)); +} + +function toMatchElement( + actualEnzymeWrapper, + reactInstance, + options = { ignoreProps: true }, +) { + const expectedWrapper = isShallowWrapper(actualEnzymeWrapper) + ? shallow(reactInstance) + : mount(reactInstance); + + const actual = actualEnzymeWrapper.debug({ verbose: true, ...options }); + const expected = expectedWrapper.debug({ verbose: true, ...options }); + const pass = actual === expected; + + return { + pass, + message: 'Expected actual value to match the expected value.', + negatedMessage: 'Did not expect actual value to match the expected value.', + contextualInformation: { + actual: `Actual:\n ${actual}`, + expected: `Expected:\n ${expected}`, + }, + }; +} + +expect.extend({ toMatchElement }); diff --git a/test/jest/setup.js b/test/jest/setup.js index 6176cfc660db..db848f08a8d2 100644 --- a/test/jest/setup.js +++ b/test/jest/setup.js @@ -1,2 +1,3 @@ // This file is for Jest-specific setup only and runs before our Jest tests. import '@testing-library/jest-dom'; +import './matchers'; diff --git a/ui/components/app/contact-list/contact-list.component.js b/ui/components/app/contact-list/contact-list.component.js index 04e3a3abd0a9..713edfcfb98d 100644 --- a/ui/components/app/contact-list/contact-list.component.js +++ b/ui/components/app/contact-list/contact-list.component.js @@ -1,5 +1,6 @@ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; +import { sortBy } from 'lodash'; import Button from '../../ui/button'; import RecipientGroup from './recipient-group/recipient-group.component'; @@ -50,34 +51,36 @@ export default class ContactList extends PureComponent { } renderAddressBook() { - const contacts = this.props.searchForContacts(); + const unsortedContactsByLetter = this.props + .searchForContacts() + .reduce((obj, contact) => { + const firstLetter = contact.name[0].toUpperCase(); + return { + ...obj, + [firstLetter]: [...(obj[firstLetter] || []), contact], + }; + }, {}); - const contactGroups = contacts.reduce((acc, contact) => { - const firstLetter = contact.name.slice(0, 1).toUpperCase(); - acc[firstLetter] = acc[firstLetter] || []; - const bucket = acc[firstLetter]; - bucket.push(contact); - return acc; - }, {}); + const letters = Object.keys(unsortedContactsByLetter).sort(); - return Object.entries(contactGroups) - .sort(([letter1], [letter2]) => { - if (letter1 > letter2) { - return 1; - } else if (letter1 === letter2) { - return 0; - } - return -1; - }) - .map(([letter, groupItems]) => ( - - )); + const sortedContactGroups = letters.map((letter) => { + return [ + letter, + sortBy(unsortedContactsByLetter[letter], (contact) => { + return contact.name.toLowerCase(); + }), + ]; + }); + + return sortedContactGroups.map(([letter, groupItems]) => ( + + )); } renderMyAccounts() { diff --git a/ui/components/app/contact-list/contact-list.test.js b/ui/components/app/contact-list/contact-list.test.js new file mode 100644 index 000000000000..8646cc70d2d3 --- /dev/null +++ b/ui/components/app/contact-list/contact-list.test.js @@ -0,0 +1,60 @@ +import React from 'react'; +import { shallowWithContext } from '../../../../test/lib/render-helpers'; +import RecipientGroup from './recipient-group'; +import ContactList from '.'; + +describe('Contact List', () => { + describe('given searchForContacts', () => { + it('sorts contacts by name within each letter group', () => { + const contacts = { + Al: { name: 'Al', address: '0x0' }, + aa: { name: 'aa', address: '0x1' }, + Az: { name: 'Az', address: '0x2' }, + Bl: { name: 'Bl', address: '0x3' }, + ba: { name: 'ba', address: '0x4' }, + Bz: { name: 'Bz', address: '0x5' }, + Ccc: { name: 'Ccc', address: '0x6' }, + }; + const searchForContacts = () => { + return Object.values(contacts); + }; + const selectRecipient = () => null; + const selectedAddress = null; + + const wrapper = shallowWithContext( + , + ); + + expect(wrapper).toMatchElement( +
+ + + +
, + { ignoreProps: false }, + ); + }); + }); +}); From 2854002f963e0574101d2d04a9b47e0ee6f2f9b0 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 3 Sep 2021 11:19:58 -0600 Subject: [PATCH 2/6] Improve tests to be less brittle --- .../app/contact-list/contact-list.test.js | 86 +++++++++---------- .../recipient-group.component.js | 15 +++- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/ui/components/app/contact-list/contact-list.test.js b/ui/components/app/contact-list/contact-list.test.js index 8646cc70d2d3..71b638dee2bd 100644 --- a/ui/components/app/contact-list/contact-list.test.js +++ b/ui/components/app/contact-list/contact-list.test.js @@ -1,60 +1,58 @@ import React from 'react'; -import { shallowWithContext } from '../../../../test/lib/render-helpers'; -import RecipientGroup from './recipient-group'; +import { within } from '@testing-library/react'; +import configureMockStore from 'redux-mock-store'; +import { renderWithProvider } from '../../../../test/jest/rendering'; import ContactList from '.'; describe('Contact List', () => { + const store = configureMockStore([])({ metamask: {} }); + describe('given searchForContacts', () => { - it('sorts contacts by name within each letter group', () => { - const contacts = { - Al: { name: 'Al', address: '0x0' }, - aa: { name: 'aa', address: '0x1' }, - Az: { name: 'Az', address: '0x2' }, - Bl: { name: 'Bl', address: '0x3' }, - ba: { name: 'ba', address: '0x4' }, - Bz: { name: 'Bz', address: '0x5' }, - Ccc: { name: 'Ccc', address: '0x6' }, - }; - const searchForContacts = () => { - return Object.values(contacts); - }; - const selectRecipient = () => null; - const selectedAddress = null; + const selectRecipient = () => null; + const selectedAddress = null; - const wrapper = shallowWithContext( + it('sorts contacts by name within each letter group', () => { + const { getAllByTestId } = renderWithProvider( { + return [ + { + name: 'Al', + address: '0x0000000000000000000000000000000000000000', + }, + { + name: 'aa', + address: '0x0000000000000000000000000000000000000001', + }, + { + name: 'Az', + address: '0x0000000000000000000000000000000000000002', + }, + { + name: 'bbb', + address: '0x0000000000000000000000000000000000000003', + }, + ]; + }} selectRecipient={selectRecipient} selectedAddress={selectedAddress} />, + store, ); - expect(wrapper).toMatchElement( -
- - - -
, - { ignoreProps: false }, + const recipientGroups = getAllByTestId('recipient-group'); + expect(within(recipientGroups[0]).getByText('A')).toBeInTheDocument(); + const recipientsInA = within(recipientGroups[0]).getAllByTestId( + 'recipient', + ); + expect(recipientsInA[0]).toHaveTextContent('aa0x0000...0001'); + expect(recipientsInA[1]).toHaveTextContent('Al0x0000...0000'); + expect(recipientsInA[2]).toHaveTextContent('Az0x0000...0002'); + expect(within(recipientGroups[1]).getByText('B')).toBeInTheDocument(); + const recipientsInB = within(recipientGroups[1]).getAllByTestId( + 'recipient', ); + expect(recipientsInB[0]).toHaveTextContent('bbb0x0000...0003'); }); }); }); diff --git a/ui/components/app/contact-list/recipient-group/recipient-group.component.js b/ui/components/app/contact-list/recipient-group/recipient-group.component.js index d5785491709c..e2f1988f51a9 100644 --- a/ui/components/app/contact-list/recipient-group/recipient-group.component.js +++ b/ui/components/app/contact-list/recipient-group/recipient-group.component.js @@ -19,9 +19,15 @@ export default function RecipientGroup({ } return ( -
+
{label && ( -
+
{label}
)} @@ -41,7 +47,10 @@ export default function RecipientGroup({ })} > -
+
{name || ellipsify(address)}
From 9a7f394e0af7e347b7cc4ab86509846f5ceb8350 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 3 Sep 2021 11:27:01 -0600 Subject: [PATCH 3/6] Remove this matcher --- test/jest/index.js | 2 -- test/jest/matchers.js | 35 ----------------------------------- test/jest/rendering.js | 4 ++-- test/jest/setup.js | 1 - 4 files changed, 2 insertions(+), 40 deletions(-) delete mode 100644 test/jest/matchers.js diff --git a/test/jest/index.js b/test/jest/index.js index b68672d34541..098877489808 100644 --- a/test/jest/index.js +++ b/test/jest/index.js @@ -1,5 +1,3 @@ -import './matchers'; - export { createSwapsMockStore } from './mock-store'; export { renderWithProvider } from './rendering'; export { setBackgroundConnection } from './background'; diff --git a/test/jest/matchers.js b/test/jest/matchers.js deleted file mode 100644 index 660018b40ed0..000000000000 --- a/test/jest/matchers.js +++ /dev/null @@ -1,35 +0,0 @@ -import { shallow, mount } from 'enzyme'; - -const SHALLOW_WRAPPER_CONSTRUCTOR = 'ShallowWrapper'; - -function isShallowWrapper(wrapper) { - return wrapper.constructor.name - ? wrapper.constructor.name === SHALLOW_WRAPPER_CONSTRUCTOR - : Boolean(`${wrapper.constructor}`.match(/^function ShallowWrapper\(/u)); -} - -function toMatchElement( - actualEnzymeWrapper, - reactInstance, - options = { ignoreProps: true }, -) { - const expectedWrapper = isShallowWrapper(actualEnzymeWrapper) - ? shallow(reactInstance) - : mount(reactInstance); - - const actual = actualEnzymeWrapper.debug({ verbose: true, ...options }); - const expected = expectedWrapper.debug({ verbose: true, ...options }); - const pass = actual === expected; - - return { - pass, - message: 'Expected actual value to match the expected value.', - negatedMessage: 'Did not expect actual value to match the expected value.', - contextualInformation: { - actual: `Actual:\n ${actual}`, - expected: `Expected:\n ${expected}`, - }, - }; -} - -expect.extend({ toMatchElement }); diff --git a/test/jest/rendering.js b/test/jest/rendering.js index f84ecfe0bc04..e843361ceae5 100644 --- a/test/jest/rendering.js +++ b/test/jest/rendering.js @@ -44,10 +44,10 @@ export function renderWithProvider(component, store) { ); return store ? ( - + ) : ( - + ); }; diff --git a/test/jest/setup.js b/test/jest/setup.js index db848f08a8d2..6176cfc660db 100644 --- a/test/jest/setup.js +++ b/test/jest/setup.js @@ -1,3 +1,2 @@ // This file is for Jest-specific setup only and runs before our Jest tests. import '@testing-library/jest-dom'; -import './matchers'; From 0e572f8a73fa7d7d790de7a9e9f1cca9c9ff6a41 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 3 Sep 2021 11:27:26 -0600 Subject: [PATCH 4/6] Revert this file --- test/jest/rendering.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jest/rendering.js b/test/jest/rendering.js index e843361ceae5..f84ecfe0bc04 100644 --- a/test/jest/rendering.js +++ b/test/jest/rendering.js @@ -44,10 +44,10 @@ export function renderWithProvider(component, store) { ); return store ? ( - + ) : ( - + ); }; From 48967e4fed3b5f92a8654970a5cefdc26da008c1 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 3 Sep 2021 11:30:51 -0600 Subject: [PATCH 5/6] Also don't need this change anymore --- .eslintrc.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index ae8b384bff5b..b6094c794e6f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -129,12 +129,7 @@ module.exports = { }, }, { - files: [ - 'ui/**/*.test.js', - 'ui/__mocks__/*.js', - 'shared/**/*.test.js', - 'test/jest/*.js', - ], + files: ['ui/**/*.test.js', 'ui/__mocks__/*.js', 'shared/**/*.test.js'], extends: ['@metamask/eslint-config-jest'], rules: { 'jest/no-restricted-matchers': 'off', From 5e933742a884543325d61a5d5ff1a3dfecacedd3 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 3 Sep 2021 11:53:59 -0600 Subject: [PATCH 6/6] Don't need this data attribute either --- .../recipient-group/recipient-group.component.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ui/components/app/contact-list/recipient-group/recipient-group.component.js b/ui/components/app/contact-list/recipient-group/recipient-group.component.js index e2f1988f51a9..a8fd589dbbe6 100644 --- a/ui/components/app/contact-list/recipient-group/recipient-group.component.js +++ b/ui/components/app/contact-list/recipient-group/recipient-group.component.js @@ -24,10 +24,7 @@ export default function RecipientGroup({ data-testid="recipient-group" > {label && ( -
+
{label}
)}