From 7cc0041aa879915eee97a915b98d60d83087e20a Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 12 Nov 2019 12:44:56 -0500 Subject: [PATCH 1/2] Remove our implementation of Chip and ChipGroup in favor of PatternFly's component --- .../components/AddRole/SelectResourceStep.jsx | 1 - .../src/components/AddRole/SelectRoleStep.jsx | 1 - awx/ui_next/src/components/Chip/Chip.jsx | 15 +- awx/ui_next/src/components/Chip/Chip.test.jsx | 2 +- awx/ui_next/src/components/Chip/ChipGroup.jsx | 44 +-- .../src/components/Chip/ChipGroup.test.jsx | 74 +--- .../src/components/Chip/CredentialChip.jsx | 2 +- .../Chip/__snapshots__/Chip.test.jsx.snap | 171 +++++++++ .../__snapshots__/ChipGroup.test.jsx.snap | 40 ++ awx/ui_next/src/components/Lookup/Lookup.jsx | 6 +- .../Lookup/MultiCredentialsLookup.test.jsx | 4 +- .../components/MultiSelect/MultiSelect.jsx | 2 +- .../ResourceAccessListItem.jsx | 8 +- .../ResourceAccessListItem.test.jsx.snap | 343 ++++++++++-------- .../components/SelectedList/SelectedList.jsx | 5 +- .../SelectedList/SelectedList.test.jsx | 11 +- .../src/screens/Job/JobDetail/JobDetail.jsx | 4 +- .../OrganizationDetail/OrganizationDetail.jsx | 2 +- .../JobTemplateDetail/JobTemplateDetail.jsx | 10 +- 19 files changed, 437 insertions(+), 308 deletions(-) create mode 100644 awx/ui_next/src/components/Chip/__snapshots__/Chip.test.jsx.snap create mode 100644 awx/ui_next/src/components/Chip/__snapshots__/ChipGroup.test.jsx.snap diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index 1128015bdd..02de0f8ee8 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -93,7 +93,6 @@ class SelectResourceStep extends React.Component { label={selectedLabel} onRemove={onRowClick} selected={selectedResourceRows} - showOverflowAfter={5} /> )} )} diff --git a/awx/ui_next/src/components/Chip/Chip.jsx b/awx/ui_next/src/components/Chip/Chip.jsx index 8c86115919..2f0a6c1049 100644 --- a/awx/ui_next/src/components/Chip/Chip.jsx +++ b/awx/ui_next/src/components/Chip/Chip.jsx @@ -4,17 +4,14 @@ import styled from 'styled-components'; Chip.displayName = 'PFChip'; export default styled(Chip)` --pf-c-chip--m-read-only--PaddingTop: 3px; - --pf-c-chip--m-read-only--PaddingRight: 8px; --pf-c-chip--m-read-only--PaddingBottom: 3px; --pf-c-chip--m-read-only--PaddingLeft: 8px; + --pf-c-chip--m-read-only--PaddingRight: 8px; - & > .pf-c-button { - padding: 3px 8px; + .pf-c-button { + --pf-c-button--PaddingTop: 3px; + --pf-c-button--PaddingBottom: 3px; + --pf-c-button--PaddingLeft: 8px; + --pf-c-button--PaddingRight: 8px; } - - ${props => - props.isOverflowChip && - ` - padding: 0; - `} `; diff --git a/awx/ui_next/src/components/Chip/Chip.test.jsx b/awx/ui_next/src/components/Chip/Chip.test.jsx index 8e38d71453..b0b9819794 100644 --- a/awx/ui_next/src/components/Chip/Chip.test.jsx +++ b/awx/ui_next/src/components/Chip/Chip.test.jsx @@ -5,6 +5,6 @@ import Chip from './Chip'; describe('Chip', () => { test('renders the expected content', () => { const wrapper = mount(); - expect(wrapper).toHaveLength(1); + expect(wrapper).toMatchSnapshot(); }); }); diff --git a/awx/ui_next/src/components/Chip/ChipGroup.jsx b/awx/ui_next/src/components/Chip/ChipGroup.jsx index 58ba8ac7e5..4098faf1f2 100644 --- a/awx/ui_next/src/components/Chip/ChipGroup.jsx +++ b/awx/ui_next/src/components/Chip/ChipGroup.jsx @@ -1,46 +1,14 @@ -import React, { useState } from 'react'; -import { number } from 'prop-types'; +import { ChipGroup } from '@patternfly/react-core'; import styled from 'styled-components'; -import Chip from './Chip'; - -const ChipGroup = ({ children, className, showOverflowAfter, ...props }) => { - const [isExpanded, setIsExpanded] = useState(!showOverflowAfter); - const toggleIsOpen = () => setIsExpanded(!isExpanded); - - const mappedChildren = React.Children.map(children, c => - React.cloneElement(c, { component: 'li' }) - ); - const showOverflowToggle = - showOverflowAfter && children.length > showOverflowAfter; - const numToShow = isExpanded - ? children.length - : Math.min(showOverflowAfter, children.length); - const expandedText = 'Show Less'; - const collapsedText = `${children.length - showOverflowAfter} more`; - - return ( -
    - {!showOverflowAfter ? mappedChildren : mappedChildren.slice(0, numToShow)} - {showOverflowAfter && showOverflowToggle && ( - - {isExpanded ? expandedText : collapsedText} - - )} -
- ); -}; -ChipGroup.propTypes = { - showOverflowAfter: number, -}; -ChipGroup.defaultProps = { - showOverflowAfter: null, -}; export default styled(ChipGroup)` --pf-c-chip-group--c-chip--MarginRight: 10px; --pf-c-chip-group--c-chip--MarginBottom: 10px; - > .pf-c-chip.pf-m-overflow button { - padding: 3px 8px; + > .pf-c-chip.pf-m-overflow .pf-c-button { + --pf-c-button--PaddingTop: 3px; + --pf-c-button--PaddingBottom: 3px; + --pf-c-chip--m-overflow--c-button--PaddingLeft: 8px; + --pf-c-chip--m-overflow--c-button--PaddingRight: 8px; } `; diff --git a/awx/ui_next/src/components/Chip/ChipGroup.test.jsx b/awx/ui_next/src/components/Chip/ChipGroup.test.jsx index f0b4a701fa..90e0dcb48f 100644 --- a/awx/ui_next/src/components/Chip/ChipGroup.test.jsx +++ b/awx/ui_next/src/components/Chip/ChipGroup.test.jsx @@ -1,74 +1,10 @@ import React from 'react'; -import { act } from 'react-dom/test-utils'; -import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; -import { ChipGroup, Chip } from '.'; +import { mount } from 'enzyme'; +import { ChipGroup } from '.'; describe('', () => { - test('should render all chips', () => { - const wrapper = mountWithContexts( - - One - Two - Three - Four - Five - Six - - ); - expect(wrapper.find(Chip)).toHaveLength(6); - expect(wrapper.find('li')).toHaveLength(6); - }); - - test('should render show more toggle', () => { - const wrapper = mountWithContexts( - - One - Two - Three - Four - Five - Six - Seven - - ); - expect(wrapper.find(Chip)).toHaveLength(6); - const toggle = wrapper.find(Chip).at(5); - expect(toggle.prop('isOverflowChip')).toBe(true); - expect(toggle.text()).toEqual('2 more'); - }); - - test('should render show less toggle', () => { - const wrapper = mountWithContexts( - - One - Two - Three - Four - Five - Six - Seven - - ); - expect(wrapper.find(Chip)).toHaveLength(6); - const toggle = wrapper.find(Chip).at(5); - expect(toggle.prop('isOverflowChip')).toBe(true); - act(() => { - toggle.prop('onClick')(); - }); - wrapper.update(); - expect(wrapper.find(Chip)).toHaveLength(8); - expect( - wrapper - .find(Chip) - .at(7) - .text() - ).toEqual('Show Less'); - act(() => { - const toggle2 = wrapper.find(Chip).at(7); - expect(toggle2.prop('isOverflowChip')).toBe(true); - toggle2.prop('onClick')(); - }); - wrapper.update(); - expect(wrapper.find(Chip)).toHaveLength(6); + test('renders the expected content', () => { + const wrapper = mount(); + expect(wrapper).toMatchSnapshot(); }); }); diff --git a/awx/ui_next/src/components/Chip/CredentialChip.jsx b/awx/ui_next/src/components/Chip/CredentialChip.jsx index 1e806a0c01..37e344bee4 100644 --- a/awx/ui_next/src/components/Chip/CredentialChip.jsx +++ b/awx/ui_next/src/components/Chip/CredentialChip.jsx @@ -4,7 +4,7 @@ import { toTitleCase } from '@util/strings'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import Chip from './Chip'; -import { Credential } from '../../types'; +import { Credential } from '@types'; function CredentialChip({ credential, i18n, ...props }) { let type; diff --git a/awx/ui_next/src/components/Chip/__snapshots__/Chip.test.jsx.snap b/awx/ui_next/src/components/Chip/__snapshots__/Chip.test.jsx.snap new file mode 100644 index 0000000000..0fc48e9943 --- /dev/null +++ b/awx/ui_next/src/components/Chip/__snapshots__/Chip.test.jsx.snap @@ -0,0 +1,171 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Chip renders the expected content 1`] = ` + + + + + + +
+ + + + + + +
+ + + + + + +`; diff --git a/awx/ui_next/src/components/Chip/__snapshots__/ChipGroup.test.jsx.snap b/awx/ui_next/src/components/Chip/__snapshots__/ChipGroup.test.jsx.snap new file mode 100644 index 0000000000..30d29d4107 --- /dev/null +++ b/awx/ui_next/src/components/Chip/__snapshots__/ChipGroup.test.jsx.snap @@ -0,0 +1,40 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders the expected content 1`] = ` + + .pf-c-chip.pf-m-overflow .pf-c-button{--pf-c-button--PaddingTop:3px;--pf-c-button--PaddingBottom:3px;--pf-c-chip--m-overflow--c-button--PaddingLeft:8px;--pf-c-chip--m-overflow--c-button--PaddingRight:8px;}", + ], + }, + "displayName": "ChipGroup", + "foldedComponentIds": Array [], + "render": [Function], + "styledComponentId": "ChipGroup-sc-10zu8t0-0", + "target": [Function], + "toString": [Function], + "warnTooManyClasses": [Function], + "withComponent": [Function], + } + } + forwardedRef={null} + > + + + +`; diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index dfe6c2ad00..ce2f85f24b 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -49,6 +49,7 @@ const InputGroup = styled(PFInputGroup)` const ChipHolder = styled.div` --pf-c-form-control--BorderTopColor: var(--pf-global--BorderColor--200); --pf-c-form-control--BorderRightColor: var(--pf-global--BorderColor--200); + --pf-c-form-control--Height: auto; border-top-right-radius: 3px; border-bottom-right-radius: 3px; `; @@ -240,7 +241,7 @@ class Lookup extends React.Component { const canDelete = !required || (multiple && value.length > 1); const chips = () => { return selectCategoryOptions && selectCategoryOptions.length > 0 ? ( - + {(multiple ? value : [value]).map(chip => ( ) : ( - + {(multiple ? value : [value]).map(chip => ( ', () => { }); test('onChange is called when you click to remove a credential from input', async () => { - const chip = wrapper.find('PFChip'); - const button = chip.at(1).find('Button'); + const chip = wrapper.find('PFChip').find({ isOverflowChip: false }); + const button = chip.at(1).find('ChipButton'); expect(chip).toHaveLength(4); button.prop('onClick')(); expect(onChange).toBeCalledWith([ diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx index 1f00008859..b8afb15855 100644 --- a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx @@ -206,7 +206,7 @@ class MultiSelect extends Component { />
- + {value.map(item => ( {userRoles.map(this.renderChip)} + + {userRoles.map(this.renderChip)} + } /> )} @@ -118,7 +120,9 @@ class ResourceAccessListItem extends React.Component { {teamRoles.map(this.renderChip)} + + {teamRoles.map(this.renderChip)} + } /> )} diff --git a/awx/ui_next/src/components/ResourceAccessList/__snapshots__/ResourceAccessListItem.test.jsx.snap b/awx/ui_next/src/components/ResourceAccessList/__snapshots__/ResourceAccessListItem.test.jsx.snap index d404aaf693..017091a61d 100644 --- a/awx/ui_next/src/components/ResourceAccessList/__snapshots__/ResourceAccessListItem.test.jsx.snap +++ b/awx/ui_next/src/components/ResourceAccessList/__snapshots__/ResourceAccessListItem.test.jsx.snap @@ -84,7 +84,9 @@ exports[` initially renders succesfully 1`] = ` fullWidth={false} label="Team Roles" value={ - + initially renders succesfully 1`] = ` fullWidth={false} label="Team Roles" value={ - + initially renders succesfully 1`] = ` fullWidth={false} label="Team Roles" value={ - + initially renders succesfully 1`] = ` fullWidth={false} label="Team Roles" value={ - + initially renders succesfully 1`] = ` className="Detail__DetailValue-sc-16ypsyv-1 yHlYM" data-pf-content={true} > - + initially renders succesfully 1`] = ` "componentStyle": ComponentStyle { "componentId": "ChipGroup-sc-10zu8t0-0", "isStatic": true, - "lastClassName": "ldAQsx", + "lastClassName": "bwbBYO", "rules": Array [ - "--pf-c-chip-group--c-chip--MarginRight:10px;--pf-c-chip-group--c-chip--MarginBottom:10px;> .pf-c-chip.pf-m-overflow button{padding:3px 8px;}", + "--pf-c-chip-group--c-chip--MarginRight:10px;--pf-c-chip-group--c-chip--MarginBottom:10px;> .pf-c-chip.pf-m-overflow .pf-c-button{--pf-c-button--PaddingTop:3px;--pf-c-button--PaddingBottom:3px;--pf-c-chip--m-overflow--c-button--PaddingLeft:8px;--pf-c-chip--m-overflow--c-button--PaddingRight:8px;}", ], }, "displayName": "ChipGroup", @@ -674,199 +684,214 @@ exports[` initially renders succesfully 1`] = ` } } forwardedRef={null} + numChips={5} >
    - - .pf-c-button{padding:3px 8px;}", - [Function], - ], - }, - "displayName": "Chip", - "foldedComponentIds": Array [], - "render": [Function], - "styledComponentId": "Chip-sc-1rzr8oo-0", - "target": [Function], - "toString": [Function], - "warnTooManyClasses": [Function], - "withComponent": [Function], - } - } - forwardedRef={null} isReadOnly={false} + key=".$3" onClick={[Function]} > - - - - -
  • - - Member - - - + Member + + - - - -
    • -     -     -     -     -     -     + + + + + + + + + + + + + + +
diff --git a/awx/ui_next/src/components/SelectedList/SelectedList.jsx b/awx/ui_next/src/components/SelectedList/SelectedList.jsx index 661eeec382..8d7c716ef9 100644 --- a/awx/ui_next/src/components/SelectedList/SelectedList.jsx +++ b/awx/ui_next/src/components/SelectedList/SelectedList.jsx @@ -23,7 +23,6 @@ class SelectedList extends Component { const { label, selected, - showOverflowAfter, onRemove, displayKey, isReadOnly, @@ -54,7 +53,7 @@ class SelectedList extends Component { {label} - {chips} + {chips} ); @@ -66,7 +65,6 @@ SelectedList.propTypes = { label: PropTypes.string, onRemove: PropTypes.func, selected: PropTypes.arrayOf(PropTypes.object).isRequired, - showOverflowAfter: PropTypes.number, isReadOnly: PropTypes.bool, }; @@ -74,7 +72,6 @@ SelectedList.defaultProps = { displayKey: 'name', label: 'Selected', onRemove: () => null, - showOverflowAfter: 5, isReadOnly: false, }; diff --git a/awx/ui_next/src/components/SelectedList/SelectedList.test.jsx b/awx/ui_next/src/components/SelectedList/SelectedList.test.jsx index e8955e077d..10a0f271b6 100644 --- a/awx/ui_next/src/components/SelectedList/SelectedList.test.jsx +++ b/awx/ui_next/src/components/SelectedList/SelectedList.test.jsx @@ -19,7 +19,6 @@ describe('', () => { {}} /> ); @@ -27,16 +26,11 @@ describe('', () => { test('showOverflow should set showOverflow on ChipGroup', () => { const wrapper = mount( - {}} - /> + {}} /> ); const chipGroup = wrapper.find(ChipGroup); expect(chipGroup).toHaveLength(1); - expect(chipGroup.prop('showOverflowAfter')).toEqual(5); + expect(chipGroup.prop('numChips')).toEqual(5); }); test('Clicking remove on chip calls onRemove callback prop with correct params', () => { @@ -51,7 +45,6 @@ describe('', () => { ); diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx index e279658c1e..47205a763d 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx @@ -217,7 +217,7 @@ function JobDetail({ job, i18n, history }) { fullWidth label={i18n._(t`Credentials`)} value={ - + {credentials.map(c => ( ))} @@ -230,7 +230,7 @@ function JobDetail({ job, i18n, history }) { fullWidth label={i18n._(t`Labels`)} value={ - + {labels.results.map(l => ( {l.name} diff --git a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx index 42594d5da9..f66b95be69 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx @@ -99,7 +99,7 @@ class OrganizationDetail extends Component { fullWidth label={i18n._(t`Instance Groups`)} value={ - + {instanceGroups.map(ig => ( {ig.name} diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 0b48128e9a..8910914179 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -273,7 +273,7 @@ class JobTemplateDetail extends Component { fullWidth label={i18n._(t`Credentials`)} value={ - + {summary_fields.credentials.map(c => ( ))} @@ -286,7 +286,7 @@ class JobTemplateDetail extends Component { fullWidth label={i18n._(t`Labels`)} value={ - + {summary_fields.labels.results.map(l => ( {l.name} @@ -301,7 +301,7 @@ class JobTemplateDetail extends Component { fullWidth label={i18n._(t`Instance Groups`)} value={ - + {instanceGroups.map(ig => ( {ig.name} @@ -316,7 +316,7 @@ class JobTemplateDetail extends Component { fullWidth label={i18n._(t`Job tags`)} value={ - + {job_tags.split(',').map(jobTag => ( {jobTag} @@ -331,7 +331,7 @@ class JobTemplateDetail extends Component { fullWidth label={i18n._(t`Skip tags`)} value={ - + {skip_tags.split(',').map(skipTag => ( {skipTag} From c13c5b6c13a56d6a084c9bbf105111ec7015a688 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Wed, 13 Nov 2019 13:45:50 -0500 Subject: [PATCH 2/2] Hide overflow chip in filter tags component --- awx/ui_next/src/components/FilterTags/FilterTags.jsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.jsx index 97773ba65a..749c326256 100644 --- a/awx/ui_next/src/components/FilterTags/FilterTags.jsx +++ b/awx/ui_next/src/components/FilterTags/FilterTags.jsx @@ -5,7 +5,7 @@ import { t } from '@lingui/macro'; import styled from 'styled-components'; import { Button } from '@patternfly/react-core'; import { parseQueryString } from '@util/qs'; -import { ChipGroup, Chip } from '@components/Chip'; +import { ChipGroup as _ChipGroup, Chip } from '@components/Chip'; import VerticalSeparator from '@components/VerticalSeparator'; const FilterTagsRow = styled.div` @@ -24,6 +24,12 @@ const FilterLabel = styled.span` padding-right: 20px; `; +const ChipGroup = styled(_ChipGroup)` + li.pf-m-overflow { + display: none; + } +`; + // remove non-default query params so they don't show up as filter tags const filterDefaultParams = (paramsArr, config) => { const defaultParamsKeys = Object.keys(config.defaultParams); @@ -66,7 +72,7 @@ const FilterTags = ({ {i18n._(t`${itemCount} results`)} {i18n._(t`Active Filters:`)} - + {queryParamsArr.map(({ key, label, value }) => (