From a1ae88cef929b10aed7c3db26c7346b7a1fb570b Mon Sep 17 00:00:00 2001 From: Jaden Diefenbaugh <jaden.diefenbaugh@idiap.ch> Date: Sun, 2 Dec 2018 13:14:34 -0800 Subject: [PATCH] fix input/output switching in tc modal, #129 --- conda/js/src/components/CacheInput.jsx | 7 +- .../components/toolchain/ToolchainModal.jsx | 54 ++++----- .../toolchain/ToolchainModal.spec.jsx | 104 +++++++++++++----- 3 files changed, 105 insertions(+), 60 deletions(-) diff --git a/conda/js/src/components/CacheInput.jsx b/conda/js/src/components/CacheInput.jsx index 5d174624..265c161a 100644 --- a/conda/js/src/components/CacheInput.jsx +++ b/conda/js/src/components/CacheInput.jsx @@ -27,6 +27,8 @@ type Props = { // update delay in ms, defaults to 500 // basically just simple throttling to keep from overwhelming complex updates in the parent delay?: number, + // danger color highlight + invalid?: boolean, }; type State = { @@ -163,6 +165,8 @@ class CacheInput extends React.Component<Props, State>{ target = null render () { + // either if invalid or if the consuming component set it + const dangerColor = !this.state.valid || this.props.invalid; return ( <InputGroup> { this.props.children } @@ -171,7 +175,8 @@ class CacheInput extends React.Component<Props, State>{ {...this.getInputProps()} value={this.state.cache} onChange={this.change} - valid={this.state.valid} + valid={!dangerColor} + invalid={dangerColor} /> { !this.state.valid && <div ref={target => {this.target = target;}} className='input-group-append'> diff --git a/conda/js/src/components/toolchain/ToolchainModal.jsx b/conda/js/src/components/toolchain/ToolchainModal.jsx index aef20d1f..aee50418 100644 --- a/conda/js/src/components/toolchain/ToolchainModal.jsx +++ b/conda/js/src/components/toolchain/ToolchainModal.jsx @@ -55,11 +55,13 @@ type Props = { export type ModalAction = 'delete' | 'add' | 'change'; +type IO = { name: string, action: ?ModalAction, original: string }; + export type ModalCache = { name: string, synchronized_channel: ?string, - inputs: ?{ name: string, action: ?ModalAction, original: string }[], - outputs: ?{ name: string, action: ?ModalAction, original: string }[], + inputs: ?IO[], + outputs: ?IO[], }; type State = { @@ -147,22 +149,20 @@ class ToolchainModal extends React.Component<Props, State> { }); } - changeIO = (io: boolean, oldName: string, newName: string) => { + changeIO = (io: boolean, oldObj: IO, newName: string) => { const arr = io ? 'inputs' : 'outputs'; - const obj = this.state.cache[arr].slice().reverse().find(obj => obj.action !== 'delete' && obj.name === oldName); - if(!obj){ - console.error(`cant find ${ oldName } in:`); - console.table(this.state.cache[arr]); - return; - //throw new Error(); - } - - const newObj = {...obj, name: newName}; + const newObj = {...oldObj, name: newName}; this.setState({ cache: { ...this.state.cache, - [arr]: this.state.cache[arr].map(o => o === obj ? newObj : o), + [arr]: this.state.cache[arr].map(o => { + if(o.original === oldObj.original){ + return newObj; + } else { + return o; + } + }), }, unsavedChanges: true, @@ -259,11 +259,11 @@ class ToolchainModal extends React.Component<Props, State> { </Button> </Col> </Row> - { unsavedChanges && + { unsavedChanges && user !== '' && name !== '' && <Row> <Col> <span className='text-danger'> - Please saved your changes before creating an algorithm! + Please save your changes before creating an algorithm! </span> </Col> </Row> @@ -327,8 +327,10 @@ class ToolchainModal extends React.Component<Props, State> { const inputNames = hasInputs ? getCurrentNames(data.inputs) : []; const outputNames = hasOutputs ? getCurrentNames(data.outputs) : []; - const hasInputDups = (new Set(inputNames)).size !== inputNames.length; - const hasOutputDups = (new Set(outputNames)).size !== outputNames.length; + const inputDup = inputNames.slice().sort().find((n, i, ns) => i !== 0 && n === ns[i - 1]); + const outputDup = outputNames.slice().sort().find((n, i, ns) => i !== 0 && n === ns[i - 1]); + const hasInputDups = inputDup != undefined; + const hasOutputDups = outputDup != undefined; return ( <Modal @@ -411,13 +413,6 @@ class ToolchainModal extends React.Component<Props, State> { </Col> } </FormGroup> - <Row> - { (hasInputDups || hasOutputDups) && - <Alert color='danger'> - Two inputs or outputs cannot share the same name! - </Alert> - } - </Row> <Row> { hasInputs && @@ -435,11 +430,9 @@ class ToolchainModal extends React.Component<Props, State> { value={input} fieldTest={true} onChange={(e) => { - this.changeIO(true, input, e.target.value); - }} - validateFunc={(str) => { - return true; + this.changeIO(true, obj, e.target.value); }} + invalid={input === inputDup} > <DeleteInputBtn deleteFunc={e => { @@ -476,10 +469,7 @@ class ToolchainModal extends React.Component<Props, State> { value={output} fieldTest={true} onChange={(e) => { - this.changeIO(false, output, e.target.value); - }} - validateFunc={(str) => { - return true; + this.changeIO(false, obj, e.target.value); }} > <DeleteInputBtn diff --git a/conda/js/src/components/toolchain/ToolchainModal.spec.jsx b/conda/js/src/components/toolchain/ToolchainModal.spec.jsx index 856ee817..cb6a8d25 100644 --- a/conda/js/src/components/toolchain/ToolchainModal.spec.jsx +++ b/conda/js/src/components/toolchain/ToolchainModal.spec.jsx @@ -12,7 +12,9 @@ const sleep = (ms) => { return new Promise(resolve => setTimeout(resolve, ms)); }; -describe('<ToolchainModal />', () => { +describe('<ToolchainModal />', function () { + this.timeout(10000); + let wrapper; afterEach(() => { @@ -31,6 +33,18 @@ describe('<ToolchainModal />', () => { synchronized_channel: 'dataset1', }; + const testBlockData2 = { + name: 'block1', + inputs: [ + 'input0', + 'input1', + ], + outputs: [ + 'output1', + ], + synchronized_channel: 'dataset1', + }; + const testChannels = { 'dataset1': '#880000' }; const testBlockNames = ['block1']; @@ -43,12 +57,9 @@ describe('<ToolchainModal />', () => { blockNames={testBlockNames} possibleChannels={testChannels} toggle={() => {}} - updateBlockName={() => {}} - updateBlockChannel={() => {}} - updateBlockIOName={() => {}} - addBlockIO={() => {}} - deleteBlockIO={() => {}} deleteBlock={() => {}} + updateBlockData={() => {}} + createAlgorithmFromBlock={() => {}} /> ); expect(wrapper).to.have.props([ @@ -57,20 +68,14 @@ describe('<ToolchainModal />', () => { 'blockNames', 'possibleChannels', 'toggle', - 'updateBlockName', - 'updateBlockChannel', - 'updateBlockIOName', - 'addBlockIO', - 'deleteBlockIO', 'deleteBlock', + 'updateBlockData', + 'createAlgorithmFromBlock', ]); }); it('lets you change the name to another valid name', async () => { const newBlockName = 'block2'; - const _updateBlockName = (str: string) => { - expect(str).to.equal(newBlockName); - }; wrapper = mount( <C active={true} @@ -78,12 +83,9 @@ describe('<ToolchainModal />', () => { blockNames={testBlockNames} possibleChannels={testChannels} toggle={() => {}} - updateBlockName={_updateBlockName} - updateBlockChannel={() => {}} - updateBlockIOName={() => {}} - addBlockIO={() => {}} - deleteBlockIO={() => {}} deleteBlock={() => {}} + updateBlockData={() => {}} + createAlgorithmFromBlock={() => {}} /> ); wrapper.find('input#tcModalInitFocus').prop('onChange')( { target: { value: newBlockName }}); @@ -95,9 +97,6 @@ describe('<ToolchainModal />', () => { it('doesnt let you change the name to a valid name with a dash ("-") in it', async () => { const newBlockName = 'block-2'; - const _updateBlockName = (str: string) => { - expect(str).to.equal(newBlockName); - }; wrapper = mount( <C active={true} @@ -105,12 +104,9 @@ describe('<ToolchainModal />', () => { blockNames={testBlockNames} possibleChannels={testChannels} toggle={() => {}} - updateBlockName={_updateBlockName} - updateBlockChannel={() => {}} - updateBlockIOName={() => {}} - addBlockIO={() => {}} - deleteBlockIO={() => {}} deleteBlock={() => {}} + updateBlockData={() => {}} + createAlgorithmFromBlock={() => {}} /> ); wrapper.find('input#tcModalInitFocus').prop('onChange')( { target: { value: newBlockName }}); @@ -119,4 +115,58 @@ describe('<ToolchainModal />', () => { wrapper.update(); expect(wrapper.exists('.input-group-append .text-danger')).to.equal(true); }); + + it('lets you switch the names of inputs (test 1)', async () => { + const newBlockName = 'block'; + wrapper = mount( + <C + active={true} + data={testBlockData2} + blockNames={testBlockNames} + possibleChannels={testChannels} + toggle={() => {}} + deleteBlock={() => {}} + updateBlockData={() => {}} + createAlgorithmFromBlock={() => {}} + /> + ); + wrapper.find('input[value="input0"]').prop('onChange')( { target: { value: 'input1' }}); + wrapper.update(); + await sleep(1000); + wrapper.find('input[value="input1"]').at(1).prop('onChange')( { target: { value: 'input0' }}); + wrapper.update(); + await sleep(1000); + wrapper.update(); + expect(wrapper.state('cache')).to.have.deep.property('inputs', [ + { action: 'change', name: 'input1', original: 'input0' }, + { action: 'change', name: 'input0', original: 'input1' }, + ]); + }); + + it('lets you switch the names of inputs (test 2)', async () => { + const newBlockName = 'block'; + wrapper = mount( + <C + active={true} + data={testBlockData2} + blockNames={testBlockNames} + possibleChannels={testChannels} + toggle={() => {}} + deleteBlock={() => {}} + updateBlockData={() => {}} + createAlgorithmFromBlock={() => {}} + /> + ); + wrapper.find('input[value="input1"]').prop('onChange')( { target: { value: 'input0' }}); + wrapper.update(); + await sleep(1000); + wrapper.find('input[value="input0"]').at(0).prop('onChange')( { target: { value: 'input1' }}); + wrapper.update(); + await sleep(1000); + wrapper.update(); + expect(wrapper.state('cache')).to.have.deep.property('inputs', [ + { action: 'change', name: 'input1', original: 'input0' }, + { action: 'change', name: 'input0', original: 'input1' }, + ]); + }); }); -- GitLab