Skip to content

Commit 16b48b9

Browse files
Jeanette Booherrdy
authored andcommitted
Use data-value attribute instead of value for an <li/>, when firing onChange call with the event followed by the value instead of mutating the target
Signed-off-by: Ryan Dy <rdy@pivotal.io>
1 parent 66e8b92 commit 16b48b9

File tree

2 files changed

+4
-5
lines changed

2 files changed

+4
-5
lines changed

spec/pivotal-ui-react/select/select_spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ describe('Select', () => {
113113

114114
it('calls then onChange callback', () => {
115115
//cannot assert on event object due to phantomJS limitation
116-
expect(onChangeSpy).toHaveBeenCalledWith(jasmine.any(Object));
116+
expect(onChangeSpy).toHaveBeenCalledWith(jasmine.any(Object), 'one');
117117
});
118118

119119
it('updates the selected value', () => {

src/react/select/select.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ export class Select extends mixin(React.Component).with(Scrim, Transition) {
3838
toggle = () => this.setState({open: !this.state.open});
3939

4040
select = e => {
41-
const value = e.target.getAttribute('value');
42-
Object.defineProperty(e.target, 'value', {value});
41+
const value = e.target.getAttribute('data-value');
4342

4443
this.setState({
4544
open: false,
4645
uncontrolledValue: value
47-
}, this.props.onChange && this.props.onChange(e));
46+
}, this.props.onChange && this.props.onChange(e, value));
4847
};
4948

5049
scrimClick = () => this.setState({open: false});
@@ -63,7 +62,7 @@ export class Select extends mixin(React.Component).with(Scrim, Transition) {
6362
className,
6463
key: value,
6564
role: 'button',
66-
value,
65+
'data-value': value,
6766
onClick: this.select
6867
}}>{label}</li>);
6968
return memo;

0 commit comments

Comments
 (0)