diff --git a/package-lock.json b/package-lock.json index 3ce8bad046..ac6628cb24 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3730,7 +3730,6 @@ }, "gulp": { "version": "git://github.com/gulpjs/gulp.git#38246c3f8b6dbb8d4ef657183e92d90c8299e22f", - "integrity": "sha1-IbaKTPujKqoeDmxc9tmMmvS1OxI=", "dev": true, "dependencies": { "camelcase": { @@ -4982,6 +4981,12 @@ "integrity": "sha1-Gmo16s5AEoDH8G3d7DUWWrJ+PlM=", "dev": true }, + "lodash.get": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", + "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", + "dev": true + }, "lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -6176,9 +6181,9 @@ "dev": true }, "react-ace": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/react-ace/-/react-ace-4.4.0.tgz", - "integrity": "sha1-be42amljyzOur8ndPCWaJGAXSSY=", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/react-ace/-/react-ace-5.0.1.tgz", + "integrity": "sha1-pqiKWygQGdPR3pJBRk1d/QLO9oo=", "dev": true }, "react-addons-test-utils": { @@ -6504,9 +6509,9 @@ "dev": true }, "require-dir": { - "version": "0.3.1", - "resolved": "https://registry.npmjs.org/require-dir/-/require-dir-0.3.1.tgz", - "integrity": "sha1-tajii64DQ7sNDMOKsfUx4ZMbJko=", + "version": "0.3.2", + "resolved": "https://registry.npmjs.org/require-dir/-/require-dir-0.3.2.tgz", + "integrity": "sha1-wdXHXp+//eny5rM+OD209ZS1pqk=", "dev": true }, "require-directory": { diff --git a/src/lib/factories.js b/src/lib/factories.js index 586221ae77..4ce9ca5768 100644 --- a/src/lib/factories.js +++ b/src/lib/factories.js @@ -77,10 +77,10 @@ export function createShorthand(Component, mapValueToProps, val, options = {}) { // ---------------------------------------- // Use key, childKey, or generate key - if (!props.key) { + if (_.isNil(props.key)) { const { childKey } = props - if (childKey) { + if (!_.isNil(childKey)) { // apply and consume the childKey props.key = typeof childKey === 'function' ? childKey(props) : childKey delete props.childKey diff --git a/src/modules/Dropdown/Dropdown.js b/src/modules/Dropdown/Dropdown.js index d1650e16ca..39643637e1 100644 --- a/src/modules/Dropdown/Dropdown.js +++ b/src/modules/Dropdown/Dropdown.js @@ -25,6 +25,8 @@ import DropdownMenu from './DropdownMenu' const debug = makeDebugger('dropdown') +const getKeyOrValue = (key, value) => _.isNil(key) ? value : key + /** * A dropdown allows a user to select a value from a series of options. * @see Form @@ -1072,7 +1074,7 @@ export default class Dropdown extends Component { ) @@ -1137,7 +1139,7 @@ export default class Dropdown extends Component { const defaultProps = { active: item.value === selectedLabel, as: 'a', - key: item.key || item.value, + key: getKeyOrValue(item.key, item.value), onClick: this.handleLabelClick, onRemove: this.handleLabelRemove, value: item.value, @@ -1168,6 +1170,7 @@ export default class Dropdown extends Component { onClick: this.handleItemClick, selected: selectedIndex === i, ...opt, + key: getKeyOrValue(opt.key, opt.value), // Needed for handling click events on disabled items style: { ...opt.style, pointerEvents: 'all' }, })) diff --git a/test/specs/lib/factories-test.js b/test/specs/lib/factories-test.js index ef69abf38a..923084e0b5 100644 --- a/test/specs/lib/factories-test.js +++ b/test/specs/lib/factories-test.js @@ -168,55 +168,119 @@ describe('factories', () => { }) }) - describe('child key', () => { - it('uses the `key` prop from an element', () => { - getShorthand({ value:
}) - .should.have.property('key', 'foo') + describe('key', () => { + it('is not consumed', () => { + getShorthand({ value: { key: 123 } }) + .props.should.have.property('key') }) - it('uses the `key` prop as a string', () => { - getShorthand({ value: { key: 'foo' } }) - .should.have.property('key', 'foo') - }) + describe('on an element', () => { + it('works with a string', () => { + getShorthand({ value:
}) + .should.have.property('key', 'foo') + }) - it('uses the `key` prop as a number', () => { - getShorthand({ value: { key: 123 } }) - .should.have.property('key', '123') + it('works with a number', () => { + getShorthand({ value:
}) + .should.have.property('key', '123') + }) + + it('works with falsy values', () => { + getShorthand({ value:
}) + .should.have.property('key', 'null') + + getShorthand({ value:
}) + .should.have.property('key', '0') + + getShorthand({ value:
}) + .should.have.property('key', '') + }) }) - it('uses the `childKey` prop as a string', () => { - getShorthand({ value: { childKey: 'foo' } }) - .should.have.property('key', 'foo') + describe('on an object', () => { + it('works with a string', () => { + getShorthand({ value: { key: 'foo' } }) + .should.have.property('key', 'foo') + }) + + it('works with a number', () => { + getShorthand({ value: { key: 123 } }) + .should.have.property('key', '123') + }) + + it('works with falsy values', () => { + getShorthand({ value: { key: null } }) + .should.have.property('key', 'null') + + getShorthand({ value: { key: 0 } }) + .should.have.property('key', '0') + + getShorthand({ value: { key: '' } }) + .should.have.property('key', '') + }) }) + }) - it('uses the `childKey` prop as a number', () => { + describe('childKey', () => { + it('is consumed', () => { getShorthand({ value: { childKey: 123 } }) - .should.have.property('key', '123') + .props.should.not.have.property('childKey') }) - it('calls `childKey` with the final `props` if it is a function', () => { - const props = { foo: 'foo', childKey: sandbox.spy(({ foo }) => foo) } + it('is called with the final `props` if it is a function', () => { + const props = { foo: 'bar', childKey: sandbox.spy(({ foo }) => foo) } const element = getShorthand({ value: props }) props.childKey.should.have.been.calledOnce() - props.childKey.should.have.been.calledWithExactly({ foo: 'foo', key: 'foo' }) + props.childKey.should.have.been.calledWithExactly({ foo: 'bar', key: 'bar' }) - element.key.should.equal('foo') + element.key.should.equal('bar') }) - it('consumes the childKey prop', () => { - getShorthand({ value: { childKey: 123 } }) - .props.should.not.have.property('childKey') - }) + describe('on an element', () => { + it('works with a string', () => { + getShorthand({ value:
}) + .should.have.property('key', 'foo') + }) + + it('works with a number', () => { + getShorthand({ value:
}) + .should.have.property('key', '123') + }) + + it('works with falsy values', () => { + getShorthand({ value:
}) + .should.have.property('key', null) - it('is generated from shorthand string values', () => { - getShorthand({ value: 'foo' }) - .should.have.property('key', 'foo') + getShorthand({ value:
}) + .should.have.property('key', '0') + + getShorthand({ value:
}) + .should.have.property('key', '') + }) }) - it('is generated from shorthand number values', () => { - getShorthand({ value: 123 }) - .should.have.property('key', '123') + describe('on an object', () => { + it('works with a string', () => { + getShorthand({ value: { childKey: 'foo' } }) + .should.have.property('key', 'foo') + }) + + it('works with a number', () => { + getShorthand({ value: { childKey: 123 } }) + .should.have.property('key', '123') + }) + + it('works with falsy values', () => { + getShorthand({ value: { childKey: null } }) + .should.have.property('key', null) + + getShorthand({ value: { childKey: 0 } }) + .should.have.property('key', '0') + + getShorthand({ value: { childKey: '' } }) + .should.have.property('key', '') + }) }) }) diff --git a/test/specs/modules/Dropdown/Dropdown-test.js b/test/specs/modules/Dropdown/Dropdown-test.js index c40ecae213..9a2730206c 100644 --- a/test/specs/modules/Dropdown/Dropdown-test.js +++ b/test/specs/modules/Dropdown/Dropdown-test.js @@ -1497,6 +1497,20 @@ describe('Dropdown', () => { .find('DropdownItem') .everyWhere(item => item.should.have.prop('data-foo', 'someValue')) }) + + it('handles keys correctly', () => { + const customOptions = [ + { key: 0, text: 'foo', value: 'foo' }, + { key: null, text: 'bar', value: 'bar' }, + { key: undefined, text: 'baz', value: 'baz' }, + ] + wrapperShallow() + const items = wrapper.find('DropdownItem') + + items.at(0).key().should.equal('0') + items.at(1).key().should.equal('bar') + items.at(2).key().should.equal('baz') + }) }) describe('selection', () => {