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

Extract option, value and single value to components and allow getting customized ones #328

Closed
wants to merge 0 commits into from

Conversation

jniechcial
Copy link

I recently bumped into case where we needed to make some customization of options and values render. I checked the options that are provided (similar to select2 formatResult) and somehow probably would deal with it this way.

However, I am pretty sure that better solution would be to wrap all these parts that often demands customization (like option/value renders) into components that can be provided via props by the user. Of course, they would need to handle proper API that should be documented.

This way, eventually react-select would become free of tens of customization props (classes, formaters, etc.). All of this could be handled directly by the user providing customized components based on default ones and handle basically every edge customization case.

What do you think? I prepared a fork that meets my requirements and is ready to merge, despite the lack of additional documentation that I can add. There is also an additional example showing how it can be useful with integrating 3rd-party components (in this example its react-gravatar.

I would love to hear your opinion.

@mxaly
Copy link

mxaly commented Jul 24, 2015

👍 for this one. Sounds like powerful solution.

@lubieniebieski
Copy link

👍

1 similar comment
@netes
Copy link

netes commented Jul 27, 2015

👍

@nekath
Copy link

nekath commented Jul 27, 2015

It looks quite good, it could solve many styling problems.
Giving thumb :)
👍

@bruderstein
Copy link
Collaborator

Be good if you could rebase this on master, and drop the "Build" commit - you only need to submit the src and test changes in a PR, otherwise there's too many conflicts.

I've tried rebasing this on my even-more-tests branch, and just one test fails - using a valueRenderer with multi=false, but that could have been my bad rebase, as it was in this area I got a conflict.

@jniechcial jniechcial closed this Aug 2, 2015
@bruderstein
Copy link
Collaborator

Why closed? :(

@jniechcial
Copy link
Author

I am currently rebasing it on other than master branch on my fork as I also got some conflicts and need to take a closer look on this to make sure it works. I will open new PR with rebased your master from my fork. Actually, I did not know that it will close the PR automatically.

@bruderstein
Copy link
Collaborator

Oh, you should just be able to force push to your old branch, and github should just update the PR - then the comments remain. Hey ho!

@jniechcial
Copy link
Author

@bruderstein I forced push to master but GH did not update it - see https://github.com/jniechcial/react-select/tree/master. It's already rebased, no conflicts, tests run and linter checked.

@jniechcial jniechcial mentioned this pull request Aug 2, 2015
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

6 participants