Fix tests 1.0 #610
Fix tests 1.0 #610
Conversation
These tests were to do with entering and leaving the options, where the behaviour is now that the option remains focussed. Probably need some new tests to cover this
This is a hangover from the old version where it produced the same result.
When the `value` prop is set to a value not in the options, previously the value would be shown as the label, now nothing is selected. Need to decide if this is a valid change.
This is as passed as a prop to `createControlWithWrapper`, but is then removed This means that onChange calls cause the value to be set.
The test for a single value of 0 was already there, and it caught the bug for the simple test of `if (!value) ...`. Yay for tests :)
onChange now called with array of option objects
There are now two ways to pass the value, either as the original object, or as the value property from the option. Both are now explicitely tested.
|
This will be the fix for #571 |
After selecting an item in a non-searchable Selectbox, the menu is no longer closed Tests changed to use this behaviour.
Fixed tests to match new behaviour.
After searching, this left the select box closed, but displaying no value, even though the value was actually selected. Tests corrected. Added `clearable` to single select example
onChange now called only with the concatenated value when simpleValue=true
Not sure if this is the right behaviour, but it does make much of the logic simpler. It is also possibly easier for a control that is part of a form, where one select influences the filtering of another. The current implementation means the filter gets called each time, which makes the logic of such a form much simpler. Tests have been amended such that the control is not initially focused, but the `filterOption` callback is still called for each option once.
Using the internal ref. This is a bit dangerous, but I can't think of a better way to do it now.
All other props are still passed through
Tests patched
This is a hangover from pre-1.0, when the current value was held in the place-holder, hence this was never noticed.
If the focus is somewhere else on the node, then the blur is ignored. This makes the "clicking away" test much simpler :)
This is now in the README, so we have fixed the test to match
These will be re-enabled / moved in a future commit
|
@JedWatson This is ready to go now - together with #609 from @MrLeebo, we're back to green \o/ - Thank you @MrLeebo for the PR! I think there's only two bug fixes - 9767a8d and 4d5eaf5 The rest, as usual, I've tried to keep each change or fix in a separate commit, so it's possible to see why each change was made. Hope that's ok. I'll open a new issue for the async tests, as I'd like to do those separately. At least we're back to green, and can work from there. There's probably one or two other bits that need tests now ( |
|
Brilliant - thank you so much @bruderstein and @MrLeebo! I promise not to do any more major test-breaking refactoring on react-select this year 😉 |
|
@JedWatson @bruderstein Glad to see this done. When I made my PR I flirted with fixing the Select tests, but I quit when I realized how much work that was going to be. Would you say this merits an NPM release? |
|
@MrLeebo I don't think there's any pressing reason to. There are only two tiny fixes, that fix edge cases (neither of which are serious), so can easily just be lumped in with the next release. Not sure when I'll get the async tests done, but when they're in, they may turn up some stuff, which would may make a release worth it. |
This is where I am up to. Please don't merge this yet, this is just to keep public where I'm up to. Should hopefully finish this weekend.