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

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

Open
WPaczula opened this issue Feb 23, 2021 · 4 comments
Open

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

WPaczula opened this issue Feb 23, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@WPaczula
Copy link

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 WPaczula added enhancement New feature or request question Further information is requested labels Feb 23, 2021
@WPaczula
Copy link
Author

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

@alexandrtovmach
Copy link
Owner

alexandrtovmach commented Feb 23, 2021

@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
Copy link
Author

WPaczula commented Feb 24, 2021

@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
Copy link
Owner

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

@alexandrtovmach alexandrtovmach removed the question Further information is requested label May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants