From 1e49984574cf46cf7d5252f177d06158aa4cd20a Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 13 Mar 2019 14:52:53 -0400 Subject: [PATCH 01/14] handleClick stop propogation, keyUp check for URL before firing onClick --- src/components/ResourceList/components/Item/Item.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/ResourceList/components/Item/Item.tsx b/src/components/ResourceList/components/Item/Item.tsx index a7b6ad5595f..ad30941d316 100644 --- a/src/components/ResourceList/components/Item/Item.tsx +++ b/src/components/ResourceList/components/Item/Item.tsx @@ -252,7 +252,7 @@ export class Item extends React.PureComponent { onFocus={this.handleFocus} onBlur={this.handleBlur} onMouseDown={this.handleMouseDown} - onKeyUp={this.handleKeypress} + onKeyUp={this.handleKeyUp} testID="Item-Wrapper" data-href={url} > @@ -321,6 +321,7 @@ export class Item extends React.PureComponent { @autobind private handleClick(event: React.MouseEvent) { + stopPropagation(event); const { id, onClick, @@ -353,15 +354,16 @@ export class Item extends React.PureComponent { } } + // This fires onClick when there is a URL on the item @autobind - private handleKeypress(event: React.KeyboardEvent) { + private handleKeyUp(event: React.KeyboardEvent) { const { onClick = noop, context: {selectMode}, } = this.props; const {key} = event; - if (key === 'Enter' && !selectMode) { + if (key === 'Enter' && this.props.url && !selectMode) { onClick(); } } From 2e2b9f8e87ab94657afccc915503677223d1d54d Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 13 Mar 2019 15:04:41 -0400 Subject: [PATCH 02/14] add unreleased.md --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 294545d159b..ed636090ce0 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -18,6 +18,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Bug fixes - Fixed unnecessary height on `TextField` due to unhandled carriage returns ([#901](https://github.com/Shopify/polaris-react/pull/901)) +- Fixed onClick from firing three times when using the enter key on a `ResourceList` item ([#1179](https://github.com/Shopify/polaris-react/pull/1179)) ### Documentation From 0b4e6f8ee5c5d423d81861740904294f6a0e6b0f Mon Sep 17 00:00:00 2001 From: Alex Page Date: Fri, 15 Mar 2019 09:34:57 -0400 Subject: [PATCH 03/14] Adding mock function for stopPropagation --- .../ResourceList/components/Item/tests/Item.test.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 67755ac4a9e..7a449103fdf 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -182,14 +182,17 @@ describe('', () => { expect(onClick).toBeCalledWith(itemId); }); - it('calls window.open on metaKey + click', () => { + it.only('calls window.open on metaKey + click', () => { const wrapper = mountWithAppProvider( , ); const item = findByTestID(wrapper, 'Item-Wrapper'); - trigger(item, 'onClick', {nativeEvent: {metaKey: true}}); + trigger(item, 'onClick', { + stopPropagation: () => {}, + nativeEvent: {metaKey: true}, + }); expect(spy).toBeCalledWith(url, '_blank'); }); @@ -200,7 +203,10 @@ describe('', () => { , ); const item = findByTestID(wrapper, 'Item-Wrapper'); - trigger(item, 'onClick', {nativeEvent: {ctrlKey: true}}); + trigger(item, 'onClick', { + stopPropagation: () => {}, + nativeEvent: {ctrlKey: true}, + }); expect(spy).toBeCalledWith(url, '_blank'); }); }); From a129fb52b768f0b91ede1d3496c0737f0665be76 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Fri, 15 Mar 2019 09:44:26 -0400 Subject: [PATCH 04/14] Remove .only my bad --- src/components/ResourceList/components/Item/tests/Item.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 7a449103fdf..02aa237f783 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -182,7 +182,7 @@ describe('', () => { expect(onClick).toBeCalledWith(itemId); }); - it.only('calls window.open on metaKey + click', () => { + it('calls window.open on metaKey + click', () => { const wrapper = mountWithAppProvider( From 7c4c23550fb4c5cb270dd5255c1df49a3a672e7a Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 20 Mar 2019 09:33:12 -0400 Subject: [PATCH 05/14] Adding test for keyUp --- .../components/Item/tests/Item.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 02aa237f783..1554aeb1274 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -196,6 +196,21 @@ describe('', () => { expect(spy).toBeCalledWith(url, '_blank'); }); + it('calls onClick when hitting keyUp on the item when onClick and URL exists', () => { + const onClick = jest.fn(); + const wrapper = mountWithAppProvider( + + + , + ); + + findByTestID(wrapper, 'Item-Wrapper').simulate('keyup', { + key: 'Enter', + }); + + expect(onClick).toHaveBeenCalled(); + }); + it('calls window.open on ctrlKey + click', () => { const wrapper = mountWithAppProvider( From 9160a582fe5df89b0b3e78a5dceac8e1105f4249 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 20 Mar 2019 09:43:16 -0400 Subject: [PATCH 06/14] Adding additional test cases --- .../components/Item/tests/Item.test.tsx | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 1554aeb1274..cf79353a4cf 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -211,6 +211,36 @@ describe('', () => { expect(onClick).toHaveBeenCalled(); }); + it('does not call onClick when hitting keyUp on non Enter key', () => { + const onClick = jest.fn(); + const wrapper = mountWithAppProvider( + + + , + ); + + findByTestID(wrapper, 'Item-Wrapper').simulate('keyup', { + key: 'Tab', + }); + + expect(onClick).not.toBeCalled(); + }); + + it('does not call onClick when hitting keyUp on the item when no URL or onClick exists', () => { + const onClick = jest.fn(); + const wrapper = mountWithAppProvider( + + + , + ); + + findByTestID(wrapper, 'Item-Wrapper').simulate('keyup', { + key: 'Enter', + }); + + expect(onClick).not.toBeCalled(); + }); + it('calls window.open on ctrlKey + click', () => { const wrapper = mountWithAppProvider( From 4bc408ccb17dcad5899435f8efda8f92236cf582 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 20 Mar 2019 09:51:12 -0400 Subject: [PATCH 07/14] Fix tsc compile issue --- .../ResourceList/components/Item/tests/Item.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index cf79353a4cf..9ca8a43d86b 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -226,11 +226,11 @@ describe('', () => { expect(onClick).not.toBeCalled(); }); - it('does not call onClick when hitting keyUp on the item when no URL or onClick exists', () => { + it('does not call onClick when hitting keyUp on the item when no URL exists', () => { const onClick = jest.fn(); const wrapper = mountWithAppProvider( - + , ); From 2e4243222bb54512cc416fa502fbc080c9613bae Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 20 Mar 2019 10:02:23 -0400 Subject: [PATCH 08/14] Add test case without onClick and only url --- .../components/Item/tests/Item.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 9ca8a43d86b..f436f5e7acf 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -241,6 +241,21 @@ describe('', () => { expect(onClick).not.toBeCalled(); }); + it('does not call onClick when hitting keyUp on the item when no onClick exists', () => { + const onClick = jest.fn(); + const wrapper = mountWithAppProvider( + + + , + ); + + findByTestID(wrapper, 'Item-Wrapper').simulate('keyup', { + key: 'Enter', + }); + + expect(onClick).not.toBeCalled(); + }); + it('calls window.open on ctrlKey + click', () => { const wrapper = mountWithAppProvider( From e5745ab0c28fa38e742a9dfafd2d5d30e95acc04 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Thu, 2 May 2019 13:16:35 -0400 Subject: [PATCH 09/14] Update UNRELEASED.md Co-Authored-By: alex-page --- UNRELEASED.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index b6bfd859afb..6da18c666d3 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -17,7 +17,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Bug fixes - Fixed unnecessary height on `TextField` due to unhandled carriage returns ([#901](https://github.com/Shopify/polaris-react/pull/901)) -- Fixed onClick from firing three times when using the enter key on a `ResourceList` item ([#1179](https://github.com/Shopify/polaris-react/pull/1179)) +- Fixed onClick from firing three times when using the enter key on a `ResourceList` item ([#1188](https://github.com/Shopify/polaris-react/pull/1188)) - Ensured server side rendering matches client side rendering for [embedded app components](https://github.com/Shopify/polaris-react/blob/master/documentation/Embedded%20apps.md#components-which-wrap-shopify-app-bridge) ([#976](https://github.com/Shopify/polaris-react/pull/976)) - Fixed disabled states while loading for `ResourceList` ([#1237](https://github.com/Shopify/polaris-react/pull/1237)) - Fixed `Checkbox` from losing focus and not receiving some modified events([#1112](https://github.com/Shopify/polaris-react/pull/1112)) From 24b8eeeb1348b77bac90c77cd421037081775f36 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Thu, 2 May 2019 13:16:47 -0400 Subject: [PATCH 10/14] Update src/components/ResourceList/components/Item/tests/Item.test.tsx Co-Authored-By: alex-page --- src/components/ResourceList/components/Item/tests/Item.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index f436f5e7acf..eec6f25a081 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -229,7 +229,7 @@ describe('', () => { it('does not call onClick when hitting keyUp on the item when no URL exists', () => { const onClick = jest.fn(); const wrapper = mountWithAppProvider( - + , ); From d63e315b5dc55402ef9503de01ae39f6991c0458 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Tue, 7 May 2019 09:01:20 -0400 Subject: [PATCH 11/14] Remove silly test --- .../components/Item/tests/Item.test.tsx | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index eec6f25a081..da37362a415 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -241,21 +241,6 @@ describe('', () => { expect(onClick).not.toBeCalled(); }); - it('does not call onClick when hitting keyUp on the item when no onClick exists', () => { - const onClick = jest.fn(); - const wrapper = mountWithAppProvider( - - - , - ); - - findByTestID(wrapper, 'Item-Wrapper').simulate('keyup', { - key: 'Enter', - }); - - expect(onClick).not.toBeCalled(); - }); - it('calls window.open on ctrlKey + click', () => { const wrapper = mountWithAppProvider( From f53be54e440d598b7d23ceae824da05b614ecd0d Mon Sep 17 00:00:00 2001 From: Alex Page Date: Tue, 7 May 2019 09:03:57 -0400 Subject: [PATCH 12/14] Removing incorrect changelogs --- UNRELEASED.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 6da18c666d3..e4a97bb76e2 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -16,9 +16,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Bug fixes -- Fixed unnecessary height on `TextField` due to unhandled carriage returns ([#901](https://github.com/Shopify/polaris-react/pull/901)) - Fixed onClick from firing three times when using the enter key on a `ResourceList` item ([#1188](https://github.com/Shopify/polaris-react/pull/1188)) -- Ensured server side rendering matches client side rendering for [embedded app components](https://github.com/Shopify/polaris-react/blob/master/documentation/Embedded%20apps.md#components-which-wrap-shopify-app-bridge) ([#976](https://github.com/Shopify/polaris-react/pull/976)) - Fixed disabled states while loading for `ResourceList` ([#1237](https://github.com/Shopify/polaris-react/pull/1237)) - Fixed `Checkbox` from losing focus and not receiving some modified events([#1112](https://github.com/Shopify/polaris-react/pull/1112)) - Added translation for the cancel button on the `ResourceList` `BulkActions` ([#1243](https://github.com/Shopify/polaris-react/pull/1243)) From a867326dc81db5c90e60878467cbf80da1ab5c55 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 8 May 2019 09:06:11 -0400 Subject: [PATCH 13/14] toBeCalledWith -> toHaveBeenCalledWith --- .../ResourceList/components/Item/tests/Item.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index 4b57b74e360..a3bef4d4854 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -194,7 +194,7 @@ describe('', () => { stopPropagation: () => {}, nativeEvent: {metaKey: true}, }); - expect(spy).toBeCalledWith(url, '_blank'); + expect(spy).toHaveBeenCalledWith(url, '_blank'); }); it('calls onClick when hitting keyUp on the item when onClick and URL exists', () => { @@ -253,7 +253,7 @@ describe('', () => { stopPropagation: () => {}, nativeEvent: {ctrlKey: true}, }); - expect(spy).toBeCalledWith(url, '_blank'); + expect(spy).toHaveBeenCalledWith(url, '_blank'); }); }); From 41587df6c262f2c31edcd0e8698ef4a2bf59c706 Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 8 May 2019 09:13:07 -0400 Subject: [PATCH 14/14] toBeCalledWith -> toHaveBeenCalled --- .../ResourceList/components/Item/tests/Item.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/ResourceList/components/Item/tests/Item.test.tsx b/src/components/ResourceList/components/Item/tests/Item.test.tsx index a3bef4d4854..21efdadc501 100644 --- a/src/components/ResourceList/components/Item/tests/Item.test.tsx +++ b/src/components/ResourceList/components/Item/tests/Item.test.tsx @@ -224,7 +224,7 @@ describe('', () => { key: 'Tab', }); - expect(onClick).not.toBeCalled(); + expect(onClick).not.toHaveBeenCalled(); }); it('does not call onClick when hitting keyUp on the item when no URL exists', () => { @@ -239,7 +239,7 @@ describe('', () => { key: 'Enter', }); - expect(onClick).not.toBeCalled(); + expect(onClick).not.toHaveBeenCalled(); }); it('calls window.open on ctrlKey + click', () => {