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

Export higher order components that can easily adapt any inputComponent to work with PhoneInput #283

Closed
TylerRick opened this issue Sep 26, 2019 · 8 comments

Comments

@TylerRick
Copy link

TylerRick commented Sep 26, 2019

It seems like this library is too biased towards its built-in input components (InputBasic and InputSmart) and makes it too hard to use other input components like those from Material UI.

It's possible to use other input components, using custom adapter components, but it just seems a lot harder than it ought to be. It took me many hours to figure out, and only after completely giving up, and then after discovering clues and progress by others (#214, #226) who have already struggled with attempting to integrate with (in my case) material-ui before me.

Ideally, everything should be "swappable": it should be trivial to swap out the default input component for any other one of your choice.

Ideally, the docs and examples should include at least one example of integration with each popular input library, such as material-ui.

Ideally, inputs from other other libraries (as well as the native <input />) should just work "out of the box".

But since I don't know exactly how to make that possible (we might want to look at how other libraries make things easily swappable for ideas), I started looking at how feasible it would be to make some general-purpose "adapter" functions that adapt other inputs so they can be used with PhoneInput.


Here's what I have working so far: https://codesandbox.io/s/integrate-react-phone-number-input-with-material-uicore-ohfwp

I've split it into the following concerns/HOCs, which you can compose as needed:

withValueFromTarget: Adapts the onChange(event: Event) API of most input components to the onChange(value: string) typing required by this library [#214]

withFocusMethod: Add a .focus() method to the component using a ref (#281 would be a better solution though)

withFormattedPhoneNumber: "Consumes" (doesn't pass on) the following props and uses them to format the input value like the default InputBasic does (#226 (comment)):

  • country : string? — The currently selected country. undefined means "International" (no country selected).
  • metadata : objectlibphonenumber-js metadata.

So basically I'm splitting up the monolithic InputBasic component into smaller functional components, because I think hooks and functional components are the future of React and are simpler, more concise, and easier to reason about.

adaptMuiInputForPhoneInput composes all of the above concerns together and lets you wrap a Material UI Input or TextField component to make it compatible.

A similar HOC should be easy to write for input or any other custom input component.

I could probably submit a pull request if there were enough interest.


Not sure where a good place for passing these default props would be:

          type="tel"
          autoComplete="tel"

but for now I've lumped it into withFormattedPhoneNumber.


Not done yet:

  • a withSmartCaret HOC that adds "smart" caret positioning using input-format library
  • a HOC that consumes/uses these:
    • onFocus() — Is used to toggle the --focus CSS class.
    • onBlur() — Is used to toggle the --focus CSS class.
@catamphetamine
Copy link
Owner

catamphetamine commented Oct 16, 2019

@TylerRick Hmmm, nice work, actually.
Didn't want to read the github email notifications until today.
Yeah, I also figured while reading your comments that the process of supplying a custom input is not as easy as it could be.
I've just implemented a new numberInputComponent property that might be a solution (will release today): bc7480c
It's "input" by default and could be set to a Material UI input or something like that.
And its onChange() function receives an event now (event.target.value should be the new value).

The HOC approach you've illustrated is an elegant way of writing a custom input component, though I though: does anyone actually need to supply a custom higher-order input component now that the "lower-order" numberInputComponent exists.
I guess the new numberInputComponent property might be enough for your case.
It's not supported for the "smart caret" mode (the smart caret itself stops working when supplied a non-default input).

Not sure where a good place for passing these default props would be: type="tel" autoComplete="tel".

Yeah, I also noticed these two.
They seem to be no longer needed (due to being overridden by the same-value properties in PhoneInput.js) so I removed them from BasicInput.js.

I think hooks and functional components are the future of React and are simpler, more concise, and easier to reason about.

I've been opposing this whole hooks thing until recently.
Now I too think that it's the new way of doing everything.

@catamphetamine
Copy link
Owner

FYI: Released [email protected] with the numberInputComponent property.

@pjay79
Copy link

pjay79 commented Nov 10, 2019

Is there an easy way to port this to React-Native? Tried using custom RN components for the numberInputComponent, and countrySelectComponent, but not sure where else to go from here...

@catamphetamine
Copy link
Owner

catamphetamine commented Nov 11, 2019

@pjay79
Did you try "without country select" component?
If not yet, post your result.
I don't think the original component could work in React Native.

@pjay79
Copy link

pjay79 commented Nov 11, 2019

@catamphetamine this is what i am doing...

import { TextInput } from 'react-native';
import PhoneInput from 'react-phone-number-input/core';
import metadata from 'libphonenumber-js/metadata.min.json';
import labels from 'react-phone-number-input/locale/default';
import InternationalIcon from 'react-phone-number-input/international-icon';

const PhoneNumberTextInput = ({ ...rest }) => {
  return (
    <TextInput
      {...rest}
      autoFocus
      keyboardType="phone-pad"
      placeholder="Fax number"
    />
  );
};

<PhoneInput
  numberInputComponent={PhoneNumberTextInput}
  labels={labels}
  metadata={metadata}
  internationalIcon={InternationalIcon}
  country="AU"
  value={this.state.number}
  onChange={this.onChangeFaxNumber}
/>

Simulator Screen Shot - iPhone X - 2019-11-11 at 11 48 43

@catamphetamine
Copy link
Owner

@pjay79 Your error is another one: some React component is undefined.
I guess it's InternationalIcon — do a console.log() and check it yourself.
If that's InternationalIcon then that would mean that there's a bug. In that case see if the default component works (not the /core one).

@pjay79
Copy link

pjay79 commented Nov 11, 2019

@catamphetamine Don't think it is undefined. See image.

Screen Shot 2019-11-11 at 12 03 26 pm

Changed the import to:

import PhoneInput from 'react-phone-number-input';

Simulator Screen Shot - iPhone X - 2019-11-11 at 12 06 59

@catamphetamine
Copy link
Owner

@pjay79 Well, that's what I was talking about: the default flagComponent renders an <img/>.
You could supply your own flagComponent.
Even with that, there's no guarantee that React Native's input supports the operations of the DOM <input/> that're used throughout the code.
I'd suggest you use "without country select" component instead.

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

No branches or pull requests

3 participants