-
Notifications
You must be signed in to change notification settings - Fork 221
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
feat: new callback prop (renderComponent) to customize rendering #395
base: master
Are you sure you want to change the base?
Conversation
if (this.props.renderComponent) { | ||
return this.props.renderComponent( | ||
wrapperProps, | ||
flagDropDownProps, | ||
inputProps, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're offloading the entire rendering work to the consumer. Correct? Have you considered a much lighter API that doesn't completely overtake the built-in rendering? Is there something that stops you from doing so?
Disclaimers:
- I have yet to test this locally.
- I can totally see where you're coming from with this sort of approach. It's a valid use case. All I'm concerned about is the complexity of the usage. The component is pretty complex as it is.
@mcataford Any opinions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look at this.
This is a purely optional additional API for more advanced users who say: yes, I know what I am doing and would like to take control of how the component should be rendered. If renderComponent()
prop is not provided, then it will still fallback on the original default rendering method, so non-expert users need not take heed of this.
The upside is that this enables the sub-components to be used in many different UI frameworks. As long as someone has figured out how to rewire this renderComponent()
adapter for use in Material UI, Reactstrap etc. they can just copy the renderComponent()
method from these examples and it is ready to use. We can put in the examples a new section on how to make this component work with some popular UI libraries to help future users so they don't have to figure it out themselves -- just copy and use.
I am not sure how I would be able to make the API lighter and still maintain the flexibility for it to potentially work with any UI library (and also not introduce any further code bloat -- the changes here are pretty minimal).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewsantarin is this PR ok for merge? It would solve my development issues.
Late to the ball on this one -- I'll read through this in a couple hours to get a proper review in. 🚀 |
Adds a new renderProp to allow user to customize how to render the component.
Description
Adds a new prop to IntlTelInput:
Currently, the rendering of the component is fixed with a
<div>
around<FlagDropDown ...>
and<input>
. If this callback is provided, instead of rendering the fixed default, will call the callback with the relevant props for user to customize their own rendering.If callback not provided, will use the existing default rendering (
<div ...> <FlagDropDown ...> <input ... >
), so it does not affect any existing usages.This allows users to customize using whatever Input component from any UI libraries (bootstrap, Material UI, etc.),. This enables this component to work smoothly with material UI (and other UI libraries) without messing around with the CSS to reproduce the UI's behaviour.
Will post CodeSandbox when I figure out how to link the module dependency to my fork...
Example usage (Material UI):
Example usage (reactstrap (bootstrap React)):
Screenshots:
Below are actual working implementation of above code:
Material UI:
Reactstrap (Bootstrap React):
Types of changes
Checklist:
Closes #391