Skip to content

Commit

Permalink
Fix double call when using committing value via keyboard
Browse files Browse the repository at this point in the history
  • Loading branch information
@nadbm
nadbm committed Jun 5, 2018
1 parent 216e331 commit 7027ad0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 36 deletions.
8 changes: 4 additions & 4 deletions lib/DataCell.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ var DataCell = function (_PureComponent) {
_this.handleContextMenu = _this.handleContextMenu.bind(_this);
_this.handleDoubleClick = _this.handleDoubleClick.bind(_this);

_this.state = { updated: false, reverting: false, value: '' };
_this.state = { updated: false, reverting: false, value: '', committing: false };
return _this;
}

Expand All @@ -107,7 +107,7 @@ var DataCell = function (_PureComponent) {
}, {
key: 'componentDidUpdate',
value: function componentDidUpdate(prevProps) {
if (prevProps.editing === true && this.props.editing === false && !this.state.reverting && this.state.value !== initialData(this.props)) {
if (prevProps.editing === true && this.props.editing === false && !this.state.reverting && !this.state.committing && this.state.value !== initialData(this.props)) {
this.props.onChange(this.props.row, this.props.col, this.state.value);
}
}
Expand All @@ -119,7 +119,7 @@ var DataCell = function (_PureComponent) {
}, {
key: 'handleChange',
value: function handleChange(value) {
this.setState({ value: value });
this.setState({ value: value, committing: false });
}
}, {
key: 'handleCommit',
Expand All @@ -129,7 +129,7 @@ var DataCell = function (_PureComponent) {
onNavigate = _props.onNavigate;

if (value !== initialData(this.props)) {
this.setState({ value: value });
this.setState({ value: value, committing: true });
onChange(this.props.row, this.props.col, value);
} else {
this.handleRevert();
Expand Down
7 changes: 4 additions & 3 deletions src/DataCell.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default class DataCell extends PureComponent {
this.handleContextMenu = this.handleContextMenu.bind(this)
this.handleDoubleClick = this.handleDoubleClick.bind(this)

this.state = {updated: false, reverting: false, value: ''}
this.state = {updated: false, reverting: false, value: '', committing: false}
}

componentWillReceiveProps (nextProps) {
Expand All @@ -53,6 +53,7 @@ export default class DataCell extends PureComponent {
if (prevProps.editing === true &&
this.props.editing === false &&
!this.state.reverting &&
!this.state.committing &&
this.state.value !== initialData(this.props)) {
this.props.onChange(this.props.row, this.props.col, this.state.value)
}
Expand All @@ -63,13 +64,13 @@ export default class DataCell extends PureComponent {
}

handleChange (value) {
this.setState({ value })
this.setState({ value, committing: false })
}

handleCommit (value, e) {
const {onChange, onNavigate} = this.props
if (value !== initialData(this.props)) {
this.setState({ value })
this.setState({ value, committing: true })
onChange(this.props.row, this.props.col, value)
} else {
this.handleRevert()
Expand Down
60 changes: 31 additions & 29 deletions test/Datasheet.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ describe('Component', () => {
wrapper.detach()
})
})

describe('rendering', () => {
it('should properly render a change (flashing)', (done) => {
const cell = {
Expand Down Expand Up @@ -407,7 +408,7 @@ describe('Component', () => {
wrapper.instance().removeAllListeners()
if (customWrapper) {
if ('removeAllListeners' in customWrapper.instance()) {
customWrapper.instance().removeAllListeners()
customWrapper.instance().removeAllListeners()
}
customWrapper = null
}
Expand Down Expand Up @@ -768,7 +769,6 @@ describe('Component', () => {
expect(customWrapper.props().selected.end).toEqual({ i: 1, j: 1 })
expect(customWrapper.find('td.cell.selected').length).toEqual(4)
})

})

describe('keyboard movement', () => {
Expand Down Expand Up @@ -1662,7 +1662,9 @@ describe('Component', () => {
handlePaste = sinon.spy()
handleCellsChanged = sinon.spy(changes => {
const newData = changeData(changes)
wrapper.setProps({data: newData})
setTimeout(() => {
wrapper.setProps({data: newData})
}, 5)
})
wrapper.setProps({onChange: handleChange, onPaste: handlePaste, onCellsChanged: handleCellsChanged})
})
Expand Down Expand Up @@ -1691,19 +1693,20 @@ describe('Component', () => {
evt.initEvent('paste', false, true)
evt.clipboardData = { getData: (type) => '99\t100\n1001\t1002'}
document.dispatchEvent(evt)
expect(handlePaste.called).toBe(false)
expect(handleCellsChanged.calledOnce).toBe(true)
expect(handleCellsChanged.firstCall.calledWith([
{cell: data[0][0], row: 0, col: 0, value: '99'},
{cell: data[0][1], row: 0, col: 1, value: '100'},
{cell: data[1][0], row: 1, col: 0, value: '1001'},
{cell: data[1][1], row: 1, col: 1, value: '1002'}
])).toBe(true)

setTimeout(() => {
expect(handlePaste.called).toBe(false)
expect(handleCellsChanged.calledOnce).toBe(true)

expect(handleCellsChanged.firstCall.calledWith([
{cell: data[0][0], row: 0, col: 0, value: '99'},
{cell: data[0][1], row: 0, col: 1, value: '100'},
{cell: data[1][0], row: 1, col: 0, value: '1001'},
{cell: data[1][1], row: 1, col: 1, value: '1002'}
])).toBe(true)
expect(handleChange.called).toBe(false)
done()
}, 1)
}, 200)
})

it('should be called with two arguments if pasted data exceeds bounds', (done) => {
Expand All @@ -1713,19 +1716,19 @@ describe('Component', () => {
evt.clipboardData = { getData: (type) => '99\t100\n1001\t1002'}
document.dispatchEvent(evt)
expect(handlePaste.called).toBe(false)
expect(handleCellsChanged.calledOnce).toBe(true)
expect(handleCellsChanged.firstCall.calledWith([
{cell: data[1][1], row: 1, col: 1, value: '99'}
], [
{row: 1, col: 2, value: '100'},
{row: 2, col: 1, value: '1001'},
{row: 2, col: 2, value: '1002'}
])).toBe(true)

setTimeout(() => {
expect(handleCellsChanged.calledOnce).toBe(true)
expect(handleCellsChanged.firstCall.calledWith([
{cell: data[1][1], row: 1, col: 1, value: '99'}
], [
{row: 1, col: 2, value: '100'},
{row: 2, col: 1, value: '1001'},
{row: 2, col: 2, value: '1002'}
])).toBe(true)
expect(handleChange.called).toBe(false)
done()
}, 1)
}, 100)
})

it('should be called once when deleting multiple cells', (done) => {
Expand All @@ -1734,18 +1737,17 @@ describe('Component', () => {

triggerKeyDownEvent(wrapper, DELETE_KEY)

expect(handleCellsChanged.calledOnce).toBe(true)
expect(handleCellsChanged.calledWith([
{cell: data[0][0], row: 0, col: 0, value: ''},
{cell: data[0][1], row: 0, col: 1, value: ''}
])).toBe(true)
expect(handlePaste.called).toBe(false)
setTimeout(() => {
expect(handleCellsChanged.calledOnce).toBe(true)
expect(handleCellsChanged.calledWith([
{cell: data[0][0], row: 0, col: 0, value: ''},
{cell: data[0][1], row: 0, col: 1, value: ''}
])).toBe(true)
expect(handlePaste.called).toBe(false)
expect(handleChange.called).toBe(false)
done()
}, 1)
}, 100)
})
})

})
})

0 comments on commit 7027ad0

Please sign in to comment.