From 0aa82c2784e93094e403dc6a3556812c31972310 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 2 Aug 2021 14:47:29 -0400 Subject: [PATCH 1/3] Adds expand all to Host List, and moved Survey list to table view --- awx/ui/src/screens/Host/HostList/HostList.js | 8 + .../src/screens/Host/HostList/HostListItem.js | 15 +- .../src/screens/Template/Survey/SurveyList.js | 63 +++-- .../screens/Template/Survey/SurveyListItem.js | 239 ++++++++---------- .../Template/Survey/SurveyListItem.test.js | 114 +++++++-- .../screens/Template/Survey/SurveyToolbar.js | 3 +- 6 files changed, 257 insertions(+), 185 deletions(-) diff --git a/awx/ui/src/screens/Host/HostList/HostList.js b/awx/ui/src/screens/Host/HostList/HostList.js index f0c03a00a5..da270228ec 100644 --- a/awx/ui/src/screens/Host/HostList/HostList.js +++ b/awx/ui/src/screens/Host/HostList/HostList.js @@ -14,6 +14,7 @@ import PaginatedTable, { } from 'components/PaginatedTable'; import useRequest, { useDeleteItems } from 'hooks/useRequest'; import useSelected from 'hooks/useSelected'; +import useExpanded from 'hooks/useExpanded'; import { encodeQueryString, getQSConfig, parseQueryString } from 'util/qs'; import HostListItem from './HostListItem'; @@ -88,6 +89,9 @@ function HostList() { const { selected, isAllSelected, handleSelect, selectAll, clearSelected } = useSelected(hosts); + const { expanded, isAllExpanded, handleExpand, expandAll } = + useExpanded(hosts); + const { isLoading: isDeleteLoading, deleteItems: deleteHosts, @@ -165,6 +169,8 @@ function HostList() { {...props} isAllSelected={isAllSelected} onSelectAll={selectAll} + isAllExpanded={isAllExpanded} + onExpandAll={expandAll} qsConfig={QS_CONFIG} additionalControls={[ ...(canAdd @@ -195,6 +201,8 @@ function HostList() { row.id === host.id)} + onExpand={() => handleExpand(host)} detailUrl={`${match.url}/${host.id}/details`} isSelected={selected.some((row) => row.id === host.id)} onSelect={() => handleSelect(host)} diff --git a/awx/ui/src/screens/Host/HostList/HostListItem.js b/awx/ui/src/screens/Host/HostList/HostListItem.js index f861a426cc..87f0bff39d 100644 --- a/awx/ui/src/screens/Host/HostList/HostListItem.js +++ b/awx/ui/src/screens/Host/HostList/HostListItem.js @@ -1,5 +1,5 @@ import 'styled-components/macro'; -import React, { useState } from 'react'; +import React from 'react'; import { string, bool, func } from 'prop-types'; import { t } from '@lingui/macro'; @@ -13,9 +13,16 @@ import HostToggle from 'components/HostToggle'; import { DetailList, Detail } from 'components/DetailList'; import Sparkline from 'components/Sparkline'; -function HostListItem({ host, isSelected, onSelect, detailUrl, rowIndex }) { +function HostListItem({ + host, + isSelected, + onSelect, + detailUrl, + rowIndex, + isExpanded, + onExpand, +}) { const labelId = `check-action-${host.id}`; - const [isExpanded, setIsExpanded] = useState(false); const { summary_fields: { recent_jobs: recentJobs = [] }, @@ -28,7 +35,7 @@ function HostListItem({ host, isSelected, onSelect, detailUrl, rowIndex }) { expand={{ rowIndex, isExpanded, - onToggle: () => setIsExpanded(!isExpanded), + onToggle: onExpand, }} /> ; } else { content = ( - - {questions?.map((question, index) => ( - q.variable === question.variable)} - onSelect={() => handleSelect(question)} - onMoveUp={moveUp} - onMoveDown={moveDown} - canEdit={canEdit} - /> - ))} + <> + + + + + {t`Name`} + {t`Type`} + {t`Default`} + {t`Actions`} + + + + {questions?.map((question, index) => ( + q.variable === question.variable + )} + onSelect={() => handleSelect(question)} + onMoveUp={moveUp} + onMoveDown={moveDown} + canEdit={canEdit} + rowIndex={index} + /> + ))} + + + {isDeleteModalOpen && deleteModal} {isPreviewModalOpen && ( )} - - + ); } diff --git a/awx/ui/src/screens/Template/Survey/SurveyListItem.js b/awx/ui/src/screens/Template/Survey/SurveyListItem.js index 75d15d31ab..818be9eb4d 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyListItem.js +++ b/awx/ui/src/screens/Template/Survey/SurveyListItem.js @@ -5,16 +5,11 @@ import { Link } from 'react-router-dom'; import { Button as _Button, Chip, - DataListAction as _DataListAction, - DataListCell, - DataListCheck, - DataListItemCells, - DataListItemRow, - DataListItem, Stack, StackItem, Tooltip, } from '@patternfly/react-core'; +import { Tr, Td } from '@patternfly/react-table'; import { CaretDownIcon, CaretUpIcon, @@ -22,20 +17,12 @@ import { } from '@patternfly/react-icons'; import styled from 'styled-components'; import ChipGroup from 'components/ChipGroup'; +import { ActionItem, ActionsTd } from 'components/PaginatedTable'; -const DataListAction = styled(_DataListAction)` - && { - margin-left: 0; - margin-right: 20px; - padding-top: 0; - padding-bottom: 0; - } -`; - -const Button = styled(_Button)` +const StackButton = styled(_Button)` padding-top: 0; padding-bottom: 0; - padding-left: 0; + padding-left: 20px; `; const Required = styled.span` @@ -43,12 +30,6 @@ const Required = styled.span` margin-left: var(--pf-global--spacer--xs); `; -const Label = styled.b` - margin-right: 20px; -`; - -const EditSection = styled(_DataListAction)``; - const EditButton = styled(_Button)``; function SurveyListItem({ @@ -60,121 +41,109 @@ function SurveyListItem({ onSelect, onMoveUp, onMoveDown, + rowIndex, }) { return ( - - - - - - - - - - - - - - - <> - - {question.question_name} - - {question.required && ( - - )} - - , - - - {question.type} - , - - - {[question.type].includes('password') && ( - {t`encrypted`.toUpperCase()} - )} - {[question.type].includes('multiselect') && - question.default.length > 0 && ( - - {question.default.split('\n').map((chip) => ( - - {chip} - - ))} - - )} - {![question.type].includes('password') && - ![question.type].includes('multiselect') && ( - {question.default} - )} - , - ]} - /> - - {canEdit && ( - - - - - - - + + + + <> + + {question.question_name} + + {question.required && ( + )} - - - + + + {question.type} + + {[question.type].includes('password') && ( + {t`encrypted`.toUpperCase()} + )} + {[question.type].includes('multiselect') && + question.default.length > 0 && ( + + {question.default.split('\n').map((chip) => ( + + {chip} + + ))} + + )} + {![question.type].includes('password') && + ![question.type].includes('multiselect') && ( + {question.default} + )} + + + + + + + + + + + + + <> + + + onMoveUp(question)} + > + + + + + onMoveDown(question)} + > + + + + + + + + ); } export default SurveyListItem; diff --git a/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js b/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js index 676f332261..0e115a1f6e 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js +++ b/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js @@ -1,6 +1,9 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; +import { + mountWithContexts, + shallowWithContexts, +} from '../../../../testUtils/enzymeHelpers'; import SurveyListItem from './SurveyListItem'; describe('', () => { @@ -11,36 +14,57 @@ describe('', () => { type: 'text', id: 1, }; + test('renders successfully', () => { let wrapper; act(() => { - wrapper = mountWithContexts( + wrapper = shallowWithContexts( ); }); expect(wrapper.length).toBe(1); }); + test('fields are rendering properly', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); const moveUp = wrapper.find('Button[aria-label="move up"]'); const moveDown = wrapper.find('Button[aria-label="move down"]'); expect(moveUp.length).toBe(1); expect(moveDown.length).toBe(1); - expect(wrapper.find('b').at(0).text()).toBe('Type'); - expect(wrapper.find('b').at(1).text()).toBe('Default'); - expect(wrapper.find('DataListCheck').length).toBe(1); - expect(wrapper.find('DataListCell').length).toBe(3); + expect(wrapper.find('SelectColumn').length).toBe(1); + expect(wrapper.find('Td').length).toBe(5); }); + test('move up and move down buttons are disabled', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); const moveUp = wrapper @@ -49,9 +73,11 @@ describe('', () => { const moveDown = wrapper .find('Button[aria-label="move down"]') .prop('isDisabled'); + expect(moveUp).toBe(true); expect(moveDown).toBe(true); }); + test('required item has required asterisk', () => { const newItem = { question_name: 'Foo', @@ -64,7 +90,17 @@ describe('', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); expect(wrapper.find('span[aria-label="Required"]').length).toBe(1); @@ -72,12 +108,19 @@ describe('', () => { test('items that are not required should not have an asterisk', () => { let wrapper; act(() => { - wrapper = mountWithContexts( - + wrapper = shallowWithContexts( + ); }); expect(wrapper.find('span[aria-label="Required"]').length).toBe(0); }); + test('required item has required asterisk', () => { const newItem = { question_name: 'Foo', @@ -89,7 +132,17 @@ describe('', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); expect(wrapper.find('Chip').length).toBe(6); @@ -98,6 +151,7 @@ describe('', () => { .filter((chip) => chip.prop('isOverFlowChip') !== true) .map((chip) => expect(chip.prop('isReadOnly')).toBe(true)); }); + test('items that are no required should have no an asterisk', () => { const newItem = { question_name: 'Foo', @@ -109,24 +163,31 @@ describe('', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); expect(wrapper.find('span').text()).toBe('ENCRYPTED'); }); + test('users without edit/delete permissions are unable to reorder the questions', () => { let wrapper; act(() => { - wrapper = mountWithContexts( + wrapper = shallowWithContexts( ); }); - expect(wrapper.find('button[aria-label="move up"]').prop('disabled')).toBe( - true - ); - expect( - wrapper.find('button[aria-label="move down"]').prop('disabled') - ).toBe(true); + expect(wrapper.find('button[aria-label="move up"]')).toHaveLength(0); + expect(wrapper.find('button[aria-label="move down"]')).toHaveLength(0); expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy(); }); @@ -134,9 +195,20 @@ describe('', () => { let wrapper; act(() => { wrapper = mountWithContexts( - + + + + +
); }); + expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy(); expect(wrapper.find('Button[ouiaId="edit-survey-buzz"]').prop('to')).toBe( 'survey/edit?question_variable=buzz' diff --git a/awx/ui/src/screens/Template/Survey/SurveyToolbar.js b/awx/ui/src/screens/Template/Survey/SurveyToolbar.js index d1a829133f..a1a31fb5fe 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyToolbar.js +++ b/awx/ui/src/screens/Template/Survey/SurveyToolbar.js @@ -16,14 +16,13 @@ import { import { ToolbarAddButton } from 'components/PaginatedTable'; const Toolbar = styled(_Toolbar)` - margin-left: 52px; + margin-left: 10px; `; function SurveyToolbar({ canEdit, isAllSelected, onSelectAll, - surveyEnabled, onToggleSurvey, isDeleteDisabled, From b4243c6f03a0f135c627787d16bc54b90f79880f Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 3 Aug 2021 15:34:28 -0400 Subject: [PATCH 2/3] adds dragging functionality to modal --- .../src/screens/Template/Survey/SurveyList.js | 69 ++--- .../Template/Survey/SurveyList.test.js | 35 +-- .../screens/Template/Survey/SurveyListItem.js | 98 ++------ .../Template/Survey/SurveyListItem.test.js | 33 --- .../Template/Survey/SurveyPreviewModal.js | 132 ---------- .../Template/Survey/SurveyReorderModal.js | 235 ++++++++++++++++++ ...dal.test.js => SurveyReorderModal.test.js} | 46 ++-- .../screens/Template/Survey/SurveyToolbar.js | 68 +++-- .../Template/Survey/SurveyToolbar.test.js | 25 +- awx/ui/src/screens/Template/Survey/index.js | 2 +- 10 files changed, 385 insertions(+), 358 deletions(-) delete mode 100644 awx/ui/src/screens/Template/Survey/SurveyPreviewModal.js create mode 100644 awx/ui/src/screens/Template/Survey/SurveyReorderModal.js rename awx/ui/src/screens/Template/Survey/{SurveyPreviewModal.test.js => SurveyReorderModal.test.js} (64%) diff --git a/awx/ui/src/screens/Template/Survey/SurveyList.js b/awx/ui/src/screens/Template/Survey/SurveyList.js index 3cd937d3e5..924ea2e8ec 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyList.js +++ b/awx/ui/src/screens/Template/Survey/SurveyList.js @@ -3,7 +3,7 @@ import React, { useState } from 'react'; import { t } from '@lingui/macro'; import { useRouteMatch } from 'react-router-dom'; import { - Button as _Button, + Button, Title, EmptyState, EmptyStateIcon, @@ -11,7 +11,6 @@ import { } from '@patternfly/react-core'; import { TableComposable, Thead, Tr, Th, Tbody } from '@patternfly/react-table'; import { CubesIcon } from '@patternfly/react-icons'; -import styled from 'styled-components'; import ContentLoading from 'components/ContentLoading'; import AlertModal from 'components/AlertModal'; import { ToolbarAddButton } from 'components/PaginatedTable'; @@ -19,12 +18,7 @@ import { ToolbarAddButton } from 'components/PaginatedTable'; import useSelected from 'hooks/useSelected'; import SurveyListItem from './SurveyListItem'; import SurveyToolbar from './SurveyToolbar'; -import SurveyPreviewModal from './SurveyPreviewModal'; - -const Button = styled(_Button)` - margin: 20px; - max-width: 85px; -`; +import SurveyReorderModal from './SurveyReorderModal'; function SurveyList({ isLoading, @@ -39,7 +33,7 @@ function SurveyList({ const questions = survey?.spec || []; const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); - const [isPreviewModalOpen, setIsPreviewModalOpen] = useState(false); + const [isOrderModalOpen, setIsOrderModalOpen] = useState(false); const { selected, isAllSelected, setSelected, selectAll, clearSelected } = useSelected(questions); @@ -62,26 +56,6 @@ function SurveyList({ clearSelected(); }; - const moveUp = (question) => { - const index = questions.indexOf(question); - if (index < 1) { - return; - } - const beginning = questions.slice(0, index - 1); - const swapWith = questions[index - 1]; - const end = questions.slice(index + 1); - updateSurvey([...beginning, question, swapWith, ...end]); - }; - const moveDown = (question) => { - const index = questions.indexOf(question); - if (index === -1 || index > questions.length - 1) { - return; - } - const beginning = questions.slice(0, index); - const swapWith = questions[index + 1]; - const end = questions.slice(index + 2); - updateSurvey([...beginning, swapWith, question, ...end]); - }; const deleteModal = ( - + - {t`Name`} - {t`Type`} - {t`Default`} - {t`Actions`} + {t`Name`} + {t`Type`} + {t`Default`} + {t`Actions`} @@ -152,27 +126,22 @@ function SurveyList({ (q) => q.variable === question.variable )} onSelect={() => handleSelect(question)} - onMoveUp={moveUp} - onMoveDown={moveDown} canEdit={canEdit} rowIndex={index} /> ))} - {isDeleteModalOpen && deleteModal} - {isPreviewModalOpen && ( - setIsPreviewModalOpen(false)} + {isOrderModalOpen && ( + setIsOrderModalOpen(false)} questions={questions} + onSave={(newOrder) => { + updateSurvey(newOrder); + setIsOrderModalOpen(false); + }} /> )} @@ -194,6 +163,12 @@ function SurveyList({ return ( <> 1 && + (() => { + setIsOrderModalOpen(true); + }) + } isAllSelected={isAllSelected} onSelectAll={selectAll} surveyEnabled={surveyEnabled} diff --git a/awx/ui/src/screens/Template/Survey/SurveyList.test.js b/awx/ui/src/screens/Template/Survey/SurveyList.test.js index 8e36ef35bb..7df31c5c00 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyList.test.js +++ b/awx/ui/src/screens/Template/Survey/SurveyList.test.js @@ -59,9 +59,10 @@ describe('', () => { }); wrapper.update(); - expect(wrapper.find('Button[variant="secondary"]').prop('isDisabled')).toBe( - true - ); + expect( + wrapper.find('Button[ouiaId="survey-delete-button"]').prop('isDisabled') + ).toBe(true); + expect(wrapper.find('Button[ouiaId="edit-order"]')).toHaveLength(1); expect( wrapper.find('Checkbox[aria-label="Select all"]').prop('isChecked') ).toBe(false); @@ -76,11 +77,11 @@ describe('', () => { expect( wrapper.find('Checkbox[aria-label="Select all"]').prop('isChecked') ).toBe(true); - expect(wrapper.find('Button[variant="secondary"]').prop('isDisabled')).toBe( - false - ); + expect( + wrapper.find('Button[ouiaId="survey-delete-button"]').prop('isDisabled') + ).toBe(false); act(() => { - wrapper.find('Button[variant="secondary"]').invoke('onClick')(); + wrapper.find('Button[ouiaId="survey-delete-button"]').invoke('onClick')(); }); wrapper.update(); @@ -91,38 +92,38 @@ describe('', () => { expect(deleteSurvey).toHaveBeenCalled(); }); - test('should render Preview button ', async () => { + test('should render Edit Order button ', async () => { let wrapper; await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); - expect(wrapper.find('Button[aria-label="Preview"]').length).toBe(1); + expect(wrapper.find('Button[ouiaId="edit-order"]').length).toBe(1); }); - test('Preview button should render Modal', async () => { + test('Edit Order button should render Modal', async () => { let wrapper; await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); - act(() => wrapper.find('Button[aria-label="Preview"]').prop('onClick')()); + act(() => wrapper.find('Button[ouiaId="edit-order"]').prop('onClick')()); wrapper.update(); - expect(wrapper.find('SurveyPreviewModal').length).toBe(1); + expect(wrapper.find('SurveyReorderModal').length).toBe(1); }); test('Modal close button should close modal', async () => { let wrapper; await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); - act(() => wrapper.find('Button[aria-label="Preview"]').prop('onClick')()); + act(() => wrapper.find('Button[ouiaId="edit-order"]').prop('onClick')()); wrapper.update(); - expect(wrapper.find('SurveyPreviewModal').length).toBe(1); + expect(wrapper.find('SurveyReorderModal').length).toBe(1); act(() => wrapper.find('Modal').prop('onClose')()); diff --git a/awx/ui/src/screens/Template/Survey/SurveyListItem.js b/awx/ui/src/screens/Template/Survey/SurveyListItem.js index 818be9eb4d..35dbe01118 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyListItem.js +++ b/awx/ui/src/screens/Template/Survey/SurveyListItem.js @@ -2,47 +2,26 @@ import 'styled-components/macro'; import React from 'react'; import { t } from '@lingui/macro'; import { Link } from 'react-router-dom'; -import { - Button as _Button, - Chip, - Stack, - StackItem, - Tooltip, -} from '@patternfly/react-core'; +import { Chip, Tooltip, Button } from '@patternfly/react-core'; + import { Tr, Td } from '@patternfly/react-table'; -import { - CaretDownIcon, - CaretUpIcon, - PencilAltIcon, -} from '@patternfly/react-icons'; +import { PencilAltIcon } from '@patternfly/react-icons'; import styled from 'styled-components'; import ChipGroup from 'components/ChipGroup'; import { ActionItem, ActionsTd } from 'components/PaginatedTable'; -const StackButton = styled(_Button)` - padding-top: 0; - padding-bottom: 0; - padding-left: 20px; -`; - const Required = styled.span` color: var(--pf-global--danger-color--100); margin-left: var(--pf-global--spacer--xs); `; -const EditButton = styled(_Button)``; +const SurveyActionsTd = styled(ActionsTd)` + && { + padding-right: 35px; + } +`; -function SurveyListItem({ - canEdit, - question, - isLast, - isFirst, - isChecked, - onSelect, - onMoveUp, - onMoveDown, - rowIndex, -}) { +function SurveyListItem({ canEdit, question, isChecked, onSelect, rowIndex }) { return ( {question.default} )} - + - - - - - - - + + + - - <> - - - onMoveUp(question)} - > - - - - - onMoveDown(question)} - > - - - - - - - + ); } diff --git a/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js b/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js index 0e115a1f6e..b93f5cc28f 100644 --- a/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js +++ b/awx/ui/src/screens/Template/Survey/SurveyListItem.test.js @@ -41,43 +41,10 @@ describe('', () => { ); }); - const moveUp = wrapper.find('Button[aria-label="move up"]'); - const moveDown = wrapper.find('Button[aria-label="move down"]'); - expect(moveUp.length).toBe(1); - expect(moveDown.length).toBe(1); expect(wrapper.find('SelectColumn').length).toBe(1); expect(wrapper.find('Td').length).toBe(5); }); - test('move up and move down buttons are disabled', () => { - let wrapper; - act(() => { - wrapper = mountWithContexts( - - - - -
- ); - }); - const moveUp = wrapper - .find('Button[aria-label="move up"]') - .prop('isDisabled'); - const moveDown = wrapper - .find('Button[aria-label="move down"]') - .prop('isDisabled'); - - expect(moveUp).toBe(true); - expect(moveDown).toBe(true); - }); - test('required item has required asterisk', () => { const newItem = { question_name: 'Foo', diff --git a/awx/ui/src/screens/Template/Survey/SurveyPreviewModal.js b/awx/ui/src/screens/Template/Survey/SurveyPreviewModal.js deleted file mode 100644 index 072af7d870..0000000000 --- a/awx/ui/src/screens/Template/Survey/SurveyPreviewModal.js +++ /dev/null @@ -1,132 +0,0 @@ -import React from 'react'; - -import { t } from '@lingui/macro'; -import { Formik } from 'formik'; - -import { - Form, - FormGroup, - Modal, - TextInput, - TextArea, - Select, - SelectOption, - SelectVariant, -} from '@patternfly/react-core'; -import { PasswordField } from 'components/FormField'; - -function SurveyPreviewModal({ - questions, - isPreviewModalOpen, - onToggleModalOpen, -}) { - const initialValues = {}; - questions.forEach((q) => { - initialValues[q.variable] = q.default; - return initialValues; - }); - - return ( - onToggleModalOpen(false)} - variant="small" - > - - {() => ( -
- {questions.map((q) => ( -
- {['text', 'integer', 'float'].includes(q.type) && ( - - - - )} - {['textarea'].includes(q.type) && ( - -