From ce1d9793ceb92b903ac8840a7e20ea975b77cd87 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Fri, 20 Sep 2019 16:25:05 -0400 Subject: [PATCH 1/4] ToolBar checkbox checks and JT Add Button closes and Test Clean up The Select-All check box in the DataList Toolbar will be checked when the user clicks on it. Also, the JT Add button closes when the user clicks elsewhere on the page as well as when the user clicks on the button a second time. I also cleaned up some tests in the DataListToolBar file. --- .../DataListToolbar/DataListToolbar.jsx | 2 +- .../DataListToolbar/DataListToolbar.test.jsx | 35 ++++++++--- .../Template/TemplateList/TemplateList.jsx | 61 ++++++++++++------- 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx index 8f8d0ecea6..0e56ee6078 100644 --- a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx +++ b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx @@ -109,7 +109,7 @@ class DataListToolbar extends React.Component { ', () => { } }); + const onSearch = jest.fn(); + const onSort = jest.fn(); + const onSelectAll = jest.fn(); + test('it triggers the expected callbacks', () => { const columns = [ { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, @@ -22,10 +26,6 @@ describe('', () => { const selectAll = 'input[aria-label="Select all"]'; const sort = 'button[aria-label="Sort"]'; - const onSearch = jest.fn(); - const onSort = jest.fn(); - const onSelectAll = jest.fn(); - toolbar = mountWithContexts( ', () => { { name: 'Baz', key: 'baz' }, ]; - const onSort = jest.fn(); - toolbar = mountWithContexts( ', () => { const columns = [ { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, ]; - const onSearch = jest.fn(); - const onSort = jest.fn(); - const onSelectAll = jest.fn(); toolbar = mountWithContexts( ', () => { expect(button).toHaveLength(1); expect(button.text()).toEqual('click'); }); + + test('it triggers the expected callbacks', () => { + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; + + toolbar = mountWithContexts( + + ); + const checkbox = toolbar.find('Checkbox'); + expect(checkbox.prop('isChecked')).toBe(true); + }); }); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 34e29329a2..e1dfa6508e 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -56,6 +56,7 @@ class TemplatesList extends Component { componentDidMount() { this.loadTemplates(); + document.addEventListener('click', this.handleAddToggle, false); } componentDidUpdate(prevProps) { @@ -65,6 +66,10 @@ class TemplatesList extends Component { } } + componentWillUnmount() { + document.removeEventListener('click', this.handleAddToggle, false); + } + handleDeleteErrorClose() { this.setState({ deletionError: null }); } @@ -84,9 +89,15 @@ class TemplatesList extends Component { } } - handleAddToggle() { + handleAddToggle(e) { const { isAddOpen } = this.state; - this.setState({ isAddOpen: !isAddOpen }); + if (this.node && this.node.contains(e.target) && isAddOpen) { + this.setState({ isAddOpen: false }); + } else if (this.node && this.node.contains(e.target) && !isAddOpen) { + this.setState({ isAddOpen: true }); + } else { + this.setState({ isAddOpen: false }); + } } async handleTemplateDelete() { @@ -215,28 +226,32 @@ class TemplatesList extends Component { itemName={i18n._(t`Template`)} />, canAdd && ( - { + this.node = node; + }} key="add" - isPlain - isOpen={isAddOpen} - position={DropdownPosition.right} - onSelect={this.handleAddSelect} - toggle={ - - } - dropdownItems={[ - - - {i18n._(t`Job Template`)} - - , - - - {i18n._(t`Workflow Template`)} - - , - ]} - /> + > + {}} />} + dropdownItems={[ + + + {i18n._(t`Job Template`)} + + , + + + {i18n._(t`Workflow Template`)} + + , + ]} + /> + ), ]} /> From 38b506bb9483b145dbeedb689f15d5f83ec55dbc Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 25 Sep 2019 09:21:56 -0400 Subject: [PATCH 2/4] Removes isPlain prop from DropdownItem --- .../src/screens/Template/TemplateList/TemplateList.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index e1dfa6508e..328e3879b5 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -239,12 +239,12 @@ class TemplatesList extends Component { onSelect={this.handleAddSelect} toggle={ {}} />} dropdownItems={[ - + {i18n._(t`Job Template`)} , - + {i18n._(t`Workflow Template`)} From d42ffd735304c22be71cd710345ec81b0df0f096 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 26 Sep 2019 10:50:15 -0400 Subject: [PATCH 3/4] Removes unused fnc and unnecessary props adds dom node to Empty State This.node was use for the add button for both empty list and list with data. This was done to reduce complexity in handleAddToggle() and I don't think it will cause bugs because those two elements are not rendered at the same time. --- .../Template/TemplateList/TemplateList.jsx | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 328e3879b5..3e69a18f8a 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -56,11 +56,11 @@ class TemplatesList extends Component { componentDidMount() { this.loadTemplates(); - document.addEventListener('click', this.handleAddToggle, false); } componentDidUpdate(prevProps) { const { location } = this.props; + if (location !== prevProps.location) { this.loadTemplates(); } @@ -91,12 +91,19 @@ class TemplatesList extends Component { handleAddToggle(e) { const { isAddOpen } = this.state; + document.addEventListener('click', this.handleAddToggle, false); + if (this.node && this.node.contains(e.target) && isAddOpen) { + + document.removeEventListener('click', this.handleAddToggle, false); this.setState({ isAddOpen: false }); + } else if (this.node && this.node.contains(e.target) && !isAddOpen) { this.setState({ isAddOpen: true }); + } else { this.setState({ isAddOpen: false }); + document.removeEventListener('click', this.handleAddToggle, false); } } @@ -236,10 +243,9 @@ class TemplatesList extends Component { isPlain isOpen={isAddOpen} position={DropdownPosition.right} - onSelect={this.handleAddSelect} - toggle={ {}} />} + toggle={} dropdownItems={[ - + {i18n._(t`Job Template`)} @@ -268,13 +274,17 @@ class TemplatesList extends Component { )} emptyStateControls={ canAdd && ( - { + this.node = node; + }} key="add" + > + } + toggle={ } dropdownItems={[ @@ -288,6 +298,7 @@ class TemplatesList extends Component { , ]} /> + ) } /> From 90cac2ec352bbfa84975c59d80ebf2bcc574bc66 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 26 Sep 2019 16:11:34 -0400 Subject: [PATCH 4/4] fix lint errors --- .../Template/TemplateList/TemplateList.jsx | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 3e69a18f8a..42b3d51090 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -94,13 +94,10 @@ class TemplatesList extends Component { document.addEventListener('click', this.handleAddToggle, false); if (this.node && this.node.contains(e.target) && isAddOpen) { - document.removeEventListener('click', this.handleAddToggle, false); this.setState({ isAddOpen: false }); - } else if (this.node && this.node.contains(e.target) && !isAddOpen) { this.setState({ isAddOpen: true }); - } else { this.setState({ isAddOpen: false }); document.removeEventListener('click', this.handleAddToggle, false); @@ -243,7 +240,9 @@ class TemplatesList extends Component { isPlain isOpen={isAddOpen} position={DropdownPosition.right} - toggle={} + toggle={ + + } dropdownItems={[ @@ -280,25 +279,25 @@ class TemplatesList extends Component { }} key="add" > - } - dropdownItems={[ - - - {i18n._(t`Job Template`)} - - , - - - {i18n._(t`Workflow Template`)} - - , - ]} - /> - + } + dropdownItems={[ + + + {i18n._(t`Job Template`)} + + , + + + {i18n._(t`Workflow Template`)} + + , + ]} + /> + ) } />