From 266349a01d1017066a88b56b1ecfe725e85f80fa Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Mon, 3 Jun 2019 15:43:44 -0700 Subject: [PATCH] refactor(core/presentation): Consolidate Checklist and ChecklistInput components --- .../serverGroup/AvailabilityZoneSelector.tsx | 10 +- .../src/forms/checklist/Checklist.spec.tsx | 129 ------------------ .../core/src/forms/checklist/Checklist.tsx | 74 ---------- app/scripts/modules/core/src/forms/index.ts | 1 - .../triggers/pipeline/PipelineTrigger.tsx | 11 +- .../forms/inputs/ChecklistInput.spec.tsx | 126 +++++++++++++++++ .../forms/inputs/ChecklistInput.tsx | 121 ++++++++++++---- 7 files changed, 227 insertions(+), 245 deletions(-) delete mode 100644 app/scripts/modules/core/src/forms/checklist/Checklist.spec.tsx delete mode 100644 app/scripts/modules/core/src/forms/checklist/Checklist.tsx create mode 100644 app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.spec.tsx diff --git a/app/scripts/modules/amazon/src/serverGroup/AvailabilityZoneSelector.tsx b/app/scripts/modules/amazon/src/serverGroup/AvailabilityZoneSelector.tsx index 5b20adb6178..20505966c01 100644 --- a/app/scripts/modules/amazon/src/serverGroup/AvailabilityZoneSelector.tsx +++ b/app/scripts/modules/amazon/src/serverGroup/AvailabilityZoneSelector.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; -import { AccountService, Checklist } from '@spinnaker/core'; +import { AccountService, ChecklistInput } from '@spinnaker/core'; export interface IAvailabilityZoneSelectorProps { region: string; @@ -93,10 +93,10 @@ export class AvailabilityZoneSelector extends React.Component< {!usePreferredZones && (
Restrict server group instances to: - ) => this.handleSelectedZonesChanged(e.target.value)} />
)} diff --git a/app/scripts/modules/core/src/forms/checklist/Checklist.spec.tsx b/app/scripts/modules/core/src/forms/checklist/Checklist.spec.tsx deleted file mode 100644 index 6ba0fdb73bf..00000000000 --- a/app/scripts/modules/core/src/forms/checklist/Checklist.spec.tsx +++ /dev/null @@ -1,129 +0,0 @@ -import * as React from 'react'; -import { shallow } from 'enzyme'; - -import { noop } from 'core/utils'; -import { Checklist } from './Checklist'; - -describe('', () => { - it('initializes properly with provided values', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow(); - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - }); - - it('updates items when an item is added externally', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow(); - - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - items.add('e'); - component.setProps({ items }); - expect(component.find('input[type="checkbox"]').length).toBe(5); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - }); - - it('updates items when an item is removed externally', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow(); - - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - items.delete('c'); - component.setProps({ items }); - expect(component.find('input[type="checkbox"]').length).toBe(3); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(2); - }); - - it('updates checked items when an item is checked externally', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow(); - - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - checked.add('d'); - component.setProps({ checked }); - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(4); - }); - - it('updates checked items when an item is unchecked externally', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow(); - - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); - checked.delete('c'); - component.setProps({ checked }); - expect(component.find('input[type="checkbox"]').length).toBe(4); - expect(component.find('input[type="checkbox"][checked=true]').length).toBe(2); - }); - - it('shows the select all button when necessary', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow( - , - ); - expect(component.find('a').length).toBe(1); - }); - - it('does not show the select all button when necessary', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow( - , - ); - expect(component.find('a').length).toBe(0); - }); - - it('shows correct text for the select all button when not all the items are checked', () => { - const checked = new Set(['a', 'b', 'c']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow( - , - ); - - expect(component.find('a').text()).toBe('Select All'); - }); - - it('shows correct text for the select all button when all the items are checked', () => { - const checked = new Set(['a', 'b', 'c', 'd']); - const items = new Set(['a', 'b', 'c', 'd']); - const component = shallow( - , - ); - - expect(component.find('a').text()).toBe('Deselect All'); - }); - - it('passes an empty list to the onChange handler when deselect all clicked', () => { - const checked = new Set(['a', 'b', 'c', 'd']); - const items = new Set(['a', 'b', 'c', 'd']); - const onChange = (i: Set): void => { - expect(i.size).toBe(0); - }; - const component = shallow( - , - ); - component.find('a').simulate('click'); - }); - - it('passes a complete list to the onChange handler when select all clicked', () => { - const checked = new Set(['a']); - const items = new Set(['a', 'b', 'c', 'd']); - const onChange = (i: Set): void => { - expect(i.size).toBe(4); - }; - const component = shallow( - , - ); - component.find('a').simulate('click'); - }); -}); diff --git a/app/scripts/modules/core/src/forms/checklist/Checklist.tsx b/app/scripts/modules/core/src/forms/checklist/Checklist.tsx deleted file mode 100644 index 9d524c7b647..00000000000 --- a/app/scripts/modules/core/src/forms/checklist/Checklist.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import * as React from 'react'; -import { xor } from 'lodash'; - -export interface IChecklistProps { - includeSelectAllButton?: boolean; - inline?: boolean; - items: Set; - checked: Set; - onChange: (checked: Set) => void; -} - -export interface IChecklistState {} - -export class Checklist extends React.Component { - constructor(props: IChecklistProps) { - super(props); - } - - private handleChecked = (event: React.ChangeEvent): void => { - const name = event.target.name; - const isChecked = event.target.checked; - const checked = new Set(this.props.checked); - - isChecked ? checked.add(name) : checked.delete(name); - this.props.onChange(checked); - }; - - public render(): React.ReactElement { - const { checked, includeSelectAllButton, inline, items, onChange } = this.props; - - const showSelectAll = includeSelectAllButton && items.size > 1; - const allSelected = xor([...items], [...checked]).length === 0; - const selectAllLabel = allSelected ? 'Deselect All' : 'Select All'; - const handleSelectAll = () => (allSelected ? onChange(new Set()) : onChange(new Set(items))); - - if (!inline) { - return ( -
    - {[...items].map(item => ( -
  • - -
  • - ))} - {showSelectAll && ( -
  • - - {selectAllLabel} - -
  • - )} -
- ); - } - - return ( - - {[...items].map(item => ( - - ))} - {showSelectAll && ( - - {selectAllLabel} - - )} - - ); - } -} diff --git a/app/scripts/modules/core/src/forms/index.ts b/app/scripts/modules/core/src/forms/index.ts index fd15d58db9a..03d3fa60ec8 100644 --- a/app/scripts/modules/core/src/forms/index.ts +++ b/app/scripts/modules/core/src/forms/index.ts @@ -1,2 +1 @@ -export * from './checklist/Checklist'; export * from './mapEditor/MapEditor'; diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/pipeline/PipelineTrigger.tsx b/app/scripts/modules/core/src/pipeline/config/triggers/pipeline/PipelineTrigger.tsx index d2adc266973..36574c984f9 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/pipeline/PipelineTrigger.tsx +++ b/app/scripts/modules/core/src/pipeline/config/triggers/pipeline/PipelineTrigger.tsx @@ -6,8 +6,7 @@ import { Observable, Subject } from 'rxjs'; import { Application, ApplicationReader } from 'core/application'; import { BaseTrigger, PipelineConfigService } from 'core/pipeline'; import { IPipeline, IPipelineTrigger } from 'core/domain'; -import { Omit } from 'core/presentation'; -import { Checklist } from 'core/forms'; +import { ChecklistInput, Omit } from 'core/presentation'; type IPipelineTriggerConfig = Omit; @@ -121,11 +120,11 @@ export class PipelineTrigger extends React.Component
- ) => this.onUpdateTrigger({ status: Array.from(s) })} + value={status} + stringOptions={this.statusOptions} + onChange={(e: React.ChangeEvent) => this.onUpdateTrigger({ status: e.target.value })} />
diff --git a/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.spec.tsx b/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.spec.tsx new file mode 100644 index 00000000000..9431ae5e820 --- /dev/null +++ b/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.spec.tsx @@ -0,0 +1,126 @@ +import * as React from 'react'; +import { mount } from 'enzyme'; + +import { ChecklistInput } from './ChecklistInput'; + +const noop = () => {}; + +describe('', () => { + it('initializes properly with provided values', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount(); + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + }); + + it('updates items when an item is added externally', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount(); + + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + component.setProps({ stringOptions: options.concat('e') }); + expect(component.find('input[type="checkbox"]').length).toBe(5); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + }); + + it('updates items when an item is removed externally', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount(); + + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + component.setProps({ stringOptions: options.filter(item => item !== 'c') }); + expect(component.find('input[type="checkbox"]').length).toBe(3); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(2); + }); + + it('updates checked items when an item is checked externally', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount(); + + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + component.setProps({ value: checkedOptions.concat('d') }); + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(4); + }); + + it('updates checked items when an item is unchecked externally', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount(); + + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(3); + component.setProps({ value: checkedOptions.filter(item => item !== 'c') }); + expect(component.find('input[type="checkbox"]').length).toBe(4); + expect(component.find('input[type="checkbox"][checked=true]').length).toBe(2); + }); + + it('shows the select all button when necessary', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount( + , + ); + expect(component.find('a').length).toBe(1); + }); + + it('does not show the select all button when necessary', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount( + , + ); + expect(component.find('a').length).toBe(0); + }); + + it('shows correct text for the select all button when not all the items are checked', () => { + const checkedOptions = ['a', 'b', 'c']; + const options = ['a', 'b', 'c', 'd']; + const component = mount( + , + ); + + expect(component.find('a').text()).toBe('Select All'); + }); + + it('shows correct text for the select all button when all the items are checked', () => { + const checkedOptions = ['a', 'b', 'c', 'd']; + const options = ['a', 'b', 'c', 'd']; + const component = mount( + , + ); + + expect(component.find('a').text()).toBe('Deselect All'); + }); + + it('passes an empty list to the onChange handler when deselect all clicked', () => { + const checkedOptions = ['a', 'b', 'c', 'd']; + const options = ['a', 'b', 'c', 'd']; + const onChange = (e: React.ChangeEvent): void => { + expect(e.target.value.length).toBe(0); + }; + const component = mount( + , + ); + component.find('a').simulate('click'); + }); + + it('passes a complete list to the onChange handler when select all clicked', () => { + const checkedOptions = ['a']; + const options = ['a', 'b', 'c', 'd']; + const onChange = (e: React.ChangeEvent): void => { + expect(e.target.value.length).toBe(4); + }; + const component = mount( + , + ); + component.find('a').simulate('click'); + }); +}); diff --git a/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.tsx b/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.tsx index 79bb202afa3..d6c38736518 100644 --- a/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.tsx +++ b/app/scripts/modules/core/src/presentation/forms/inputs/ChecklistInput.tsx @@ -1,11 +1,13 @@ import * as React from 'react'; +import { IFormInputProps, OmitControlledInputPropsFrom } from '../interface'; import { createFakeReactSyntheticEvent, isStringArray, orEmptyString, validationClassName } from './utils'; -import { IFormInputProps, OmitControlledInputPropsFrom } from '../interface'; interface IChecklistInputProps extends IFormInputProps, OmitControlledInputPropsFrom> { options?: IChecklistInputOption[]; stringOptions?: string[]; + inline?: boolean; + showSelectAll?: boolean; } export interface IChecklistInputOption { @@ -13,43 +15,102 @@ export interface IChecklistInputOption { value: string; } -export class ChecklistInput extends React.Component { - public render() { - const { value, validation, inputClassName, options, stringOptions, onChange, ...otherProps } = this.props; - const className = `${orEmptyString(inputClassName)} ${validationClassName(validation)}`; +export function ChecklistInput(props: IChecklistInputProps) { + const { + inline, + showSelectAll, + value, + validation, + inputClassName, + options, + stringOptions, + onChange, + ...otherProps + } = props; + + const className = `${orEmptyString(inputClassName)} ${validationClassName(validation)}`; + + const selectedValues = value || []; + const isChecked = (checkboxValue: any) => selectedValues.includes(checkboxValue); + + const checkListOptions = isStringArray(stringOptions) + ? stringOptions.map(s => ({ label: s, value: s })) + : options || []; + + const labelClassName = !!inline ? 'clickable checkbox-inline' : 'clickable'; + + function CheckBox(checkboxProps: { option: IChecklistInputOption }) { + const { option } = checkboxProps; - const handleChange = (e: React.ChangeEvent) => { + function handleChange(e: React.ChangeEvent) { const selected = e.target.value; - let newValue = value.concat(selected); - if (value.includes(selected)) { - newValue = value.filter((v: string) => v !== selected); - } + const alreadyHasValue = isChecked(selected); + const newValue = !alreadyHasValue ? selectedValues.concat(selected) : value.filter((v: string) => v !== selected); onChange(createFakeReactSyntheticEvent({ value: newValue, name: e.target.name })); + } + + return ( + + ); + } + + function SelectAllButton() { + const allSelected = checkListOptions.every(option => isChecked(option.value)); + const anchorClassName = `btn btn-default btn-xs ${inline ? '' : 'push-left'}`; + const style = inline ? { margin: '8px 0 0 10px' } : {}; + + const selectValue = (selected: string[]) => { + return onChange(createFakeReactSyntheticEvent({ name: props.name, value: selected })); }; + const selectNone = () => selectValue([]); + const selectAll = () => selectValue(checkListOptions.map(o => o.value)); - let checkListOptions = options || []; - if (isStringArray(stringOptions)) { - checkListOptions = stringOptions.map(s => ({ label: s, value: s })); - } + return ( + + {allSelected ? 'Deselect All' : 'Select All'} + + ); + } + function InlineOptions() { return ( -
- {checkListOptions.map(o => ( -
- -
+ <> + {checkListOptions.map(option => ( + ))} -
+ + {showSelectAll && checkListOptions.length > 1 && } + ); } + + function VerticalOptions() { + return ( +
    + {checkListOptions.map(option => ( +
  • + +
  • + ))} + + {showSelectAll && checkListOptions.length > 1 && ( +
  • + +
  • + )} +
+ ); + } + + return
{inline ? : }
; }