Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only display options list if not empty #112

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

magalhas
Copy link

@magalhas magalhas commented Aug 3, 2015

When there were no visible items the

    was being rendered without need.

@fmoo
Copy link
Owner

fmoo commented Aug 3, 2015

I haven't read through the rest of the code lately, but does this break anything that assumes "sel" is a valid reference anywhere else?

@magalhas
Copy link
Author

magalhas commented Aug 3, 2015

From my experience it's not throwing any errors, but yeah the ref won't be available with this if statement.

I wanted to put this logic inside the List component, though you can't return undefined / null on a render method.

@magalhas
Copy link
Author

It would be awesome if this could be included so that we can stop using our fork git dependency.

@fmoo
Copy link
Owner

fmoo commented Mar 10, 2016

Can you rebase and move the logic into _shouldSkipSearch() ?

@magalhas
Copy link
Author

@fmoo done.

@fmoo
Copy link
Owner

fmoo commented Mar 11, 2016

Hey, we've had a couple of regressions lately, would you mind including a unit test for this as well?

@magalhas
Copy link
Author

Sure, not sure when I'll be able to pick this up though. What about my other 2 PRs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants