alexandrtovmach / react-microsoft-login

Microsoft services authorization with React.
https://alexandrtovmach.github.io/react-microsoft-login
MIT License
80 stars 38 forks source link

Use render props to pass onClick handler to a custom button #74

Open WPaczula opened 3 years ago

WPaczula commented 3 years ago

Is your feature request related to a problem? Please describe. I'd like to style the login button in a custom way. This library creates an additional div with onClick handler and there is no way to apply class name to it in a clean way.

Describe the solution you'd like More flexible solution could be a usage of render props with onClick handler as a prop called render

Describe alternatives you've considered Other option would be to adjust children prop to allow functions. If the function is passed => invoke it with onClick prop, if it's a jsx element render it in div not to make a breaking change

Additional context Proposed API:


<MicrosoftLogin 
    clientId={'id'}
    authCallback={authCallback}
    render={({ onClick }) => <MyCustomComponent onClick={onClick} />}
/>
WPaczula commented 3 years ago

If my solution is fine for you, I can contribute for this repo, that should be a quick thing to do 🚀

alexandrtovmach commented 3 years ago

@WPaczula thanks for reporting It's already done by children prop. Something like this:

<MicrosoftLogin 
    clientId={'id'}
    authCallback={authCallback}
>
  <MyCustomComponent />
</MicrosoftLogin>

Passing custom onClick method doesn't sound reasonable, because how MicrosoftLogin should detect click, if you passed your own?

WPaczula commented 3 years ago

@alexandrtovmach thanks for the replay 😄. onClick method would be the login function from this piece of code.

I propose using render props pattern, which would look like this:

if (render) {
   return render({ onClick: login });
} else {
    return children ? (
    <div onClick={login}>{children}</div>
  ) : (
    <MicrosoftLoginButton
      buttonTheme={buttonTheme || "light"}
      buttonClassName={className}
      onClick={login}
    />
  );
}

Structuring the code this way would allow the consumers of this library to inform it that the login button was clicked, but it is decoupled from actual implementation of handling the click. They would be able to use it however they like without any impact on their html.

alexandrtovmach commented 3 years ago

Sorry for delay =) Yes, it sounds reasonable, so if you still want, I'd happy to see your PR with this updates