fix(numericSelector): make default value possible #2565
Conversation
|
Deploy preview ready! Built with commit c6771b7 https://deploy-preview-2565--algolia-instantsearch.netlify.com |
| @@ -235,4 +235,57 @@ describe('connectNumericSelector', () => { | |||
| refine = renderingParameters.refine; | |||
| }); | |||
| }); | |||
|
|
|||
| it('The refine function can unselect with `undefined` and "undefined"', () => { | |||
| const rendering = sinon.stub(); | |||
There was a problem hiding this comment.
Prefer usage of jest.fn, at some point we should remove sinon
There was a problem hiding this comment.
Yes but if I move to jest.fn, I need to do the whole file. You might say "yes exactly do it and move like that each time you encounter sinon" and I would have to agree. OK :)
| @@ -20,13 +20,13 @@ export class RawSelector extends React.Component { | |||
| <select | |||
| className={this.props.cssClasses.select} | |||
| onChange={this.handleChange} | |||
| value={currentValue} | |||
| value={`${currentValue}`} | |||
There was a problem hiding this comment.
Why did you set them to string? If we keep value as number, it's still works. The coercion is made by the browser.
There was a problem hiding this comment.
I will try then. :)
There was a problem hiding this comment.
Oh I know, I have to transform to string because of undefined...
There was a problem hiding this comment.
Yes you are right... Maybe at some point we should also support empty string as reset value. With it we don't need to cast number to string in the template, just provide the default value as "".
There was a problem hiding this comment.
Well I think undefined is the most natural move since you can completely avoid the value and I don't think we should support more than one "0" value. However we could change it to "" if you and the team think it's best.
When we created the connectNumericSelector, we made sure to support
undefinedas a special value to unset the filters. However because the implementations are DOM based, the value is now a string. This patch fixes that (and adds the documentation to the feature)I also updated the example because it was super hard to maintain and didn't have much sense in terms of feature. The new example also show case the undefined value :)