feat: render the search input inside the dropdown

[ingmothman]

What does it do?

Allows us to show/hide the search input and change the location from the dropdown toggle to dropdown content.

Fixes #308

Type of change

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• Updated documentation (if applicable)
• Added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• My changes generate no new warnings

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mrchief]

@osmancode Looks like I don't have push rights to this PR. I wanted to note a few things I found:

• remove package-lock.json and update yarn.lock. This project uses yarn for dev, so we need to continue that.
• the component width differs with inline search and no selections. I'm not sure if this is an issue or not. It seems like an artifact of how default browser styling happens and this is something one can fix using CSS, so I guess it's a non issue.

normal	inline

Everything else looks OK to me.

P.S.: A bit of trivia, I've added and removed Tags so many times in various experimental branches. Something always bothered me about it but I couldn't find enough justification to make it its own component without making it look like one, just for the sake of it. But now we have a solid justification!

[mrchief]

!tags || (tags && tags.length === 0)

Seems like a bit of excessive branching. Can we use ES6 defaults and eliminate branches or better yet, eliminate it altogether since it seems redundant.

{!children && (!tags || (tags && tags.length === 0)) && (
  <li className={cx('tag-item')}>
    <span className={cx('placeholder')}>{texts.placeholder || 'Choose...'}</span>
  </li>
)}

If no children, you'd render the placeholder regardless of tags or no tags, no?

I was wondering if we could do something like:

const lastItem = children || <span className={cx('placeholder')}>{texts.placeholder || 'Choose...'}</span>
return (
  <ul className={cx('tag-list')}>
    {getTags(tags, onTagRemove, readOnly, disabled, texts.labelRemove)}
    <li className={cx('tag-item')}>{lastItem}</li>
  </ul>
)

and maybe even inline getTags since there's no good reason for it to exist outside (it was done earlier to reduce code complexity when it was inside Input's render block).

Thoughts?

[ingmothman]

@mrchief i have granted you push permissions over the my repo.

[mrchief]

@osmancode were you able to work on these changes?

[ingmothman]

@mrchief i would like to but unfortunately i will be very busy for the next two months. do u think you will be able to finalize the PR?

[mrchief]

I'll try to... just didn't wanna duplicate efforts if you're already on it.

[mrchief]

@osmancode I think stale bot closed this and if that's the case, I'd need to check its settings as it wasn't supposed to close wip PRs.

[codeclimate]

The PR diff size of 25795 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

Code Climate has analyzed commit ef58bef and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

View more on Code Climate.

[mrchief]

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀