Searching text prop (Rebased #141) #379
Conversation
…age while searching asynchronously.
Removed IIFE, as there was no advantage to wrapping the `if` statement in an IIFE.
We were abusing .Select-noresults to also hold the `Searching...` and `searchPromptText`. This commit fixes that, so that each type of text has its own className. This makes JedWatson#221 redundant, as you can hide the div with a simple css rule, if you want to hide it.
|
@bruderstein looks good! I personally find that the less indentation and syntax there is, the less I have to grok in my head, but I guess that is just a style thing 👍 , it was a nit, my main gripe was the unnecessary IIFE. Thanks for rebasing! |
| @@ -2389,6 +2389,69 @@ describe('Select', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('searchingText', function () { | |||
|
|
|||
There was a problem hiding this comment.
Note this function padding will end up failing happiness once the bug is fixed in eslint.
There was a problem hiding this comment.
because of the blank line after the function () {?
Eeee. Funnily enough, that's the hapi style, which I like, and find it makes it much easier to read, I also saw that there is a blank line after many functions.
We'll need to do a clean up of all the tests if that gets accepted. I really don't like that it seems to fail on node 0.10 though.
|
This looks great, thanks for cleaning it up @bruderstein! |
Searching text prop (Rebased #141)
This is a rebased version of #141 from @johnomalley, with some tests added.
This makes sense to me, if you change
autoloadtofalsein the remote options example, and typea, you getNo resultsand a spinner for half a second, then the results come in. With the patch, it showsSearching..., customisable by thesearchingTextprop.I've left the code pretty much as is (I'm with @johnomalley on it, the
ifmakes it more readable). I've removed the IIFE though, as there doesn't seem any advantageous, and creates another function scope and another bind - feel free to show where I'm missing something.Tests for the new functionality added in a separate commit.