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

Add avatar icon as login property w/ default. #9917

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

mjarosch
Copy link

@mjarosch mjarosch commented Jun 11, 2024

This is a pull request for issue #9916. It adds an avatarIcon property to the Login component w/ a default of the Lock icon. This allows for overriding the icon without having to copy the complete component.

Ideally, this would be included as part of a v4 point release. This is my first pull request, so please let me know if there is anything else needed.

--

Fix #9916

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, which I approve.

However, since this is a new feature, it needs to target the next branch. Can you rebase?

As a reminder, React Admin v4 is now feature-freeze, which means this new feature will have to be released with React Admin v5.

Thanks again for your contribution!

@mjarosch mjarosch changed the base branch from master to next June 12, 2024 11:40
@mjarosch mjarosch force-pushed the feature/login-avatar-icon branch 4 times, most recently from a91a5be to 7755320 Compare June 12, 2024 11:54
@mjarosch
Copy link
Author

@slax57 Branch has been rebased.

@djhi
Copy link
Contributor

djhi commented Jun 12, 2024

Would you mind adding a story for it? We don't have any for this component so take inspiration from the Logout.stories.tsx. Thanks!

@slax57
Copy link
Contributor

slax57 commented Jul 1, 2024

It seems your branch now includes too many commits. Looks like you messed up the rebase 😬 . Can you fix the branch? 🙏

@mjarosch
Copy link
Author

mjarosch commented Jul 1, 2024

@slax57 Not sure what happened, but the branch has been cleaned up. I still plan on getting a story added for this, I just have been busy and haven't had time to get to it.

@slax57
Copy link
Contributor

slax57 commented Jul 1, 2024

No worries, we'll wait on the story then. Thanks!

@mjarosch
Copy link
Author

mjarosch commented Jul 4, 2024

@slax57 @djhi Sorry for taking so long to get the stories added. Everything should be ready now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the icon on the login screen a property so it can be customized
4 participants