-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Pick icons on login screen by connector type instead of ID #1576
Conversation
With this PR change, how to deals with multiple LDAP connectors? |
That's not quite true. There is a google provider now.
This PR only changes the logo, but not the provider name, so I think it should continue working just fine. @nabokihms can you please rebase your branch from master to make GitHub actions happy? |
Thanks, I got the point ;) |
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.
LGTM
3a8cb83
to
db631b7
Compare
@sagikazarmark rebased :) |
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 @nabokihms !
There is one small thing I would change, otherwise it is good to go. Good catch!
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
98879e2
to
058e72e
Compare
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 @nabokihms !
We are picking icons by provider ID. For example:
Using type field will make user always have icons for all providers on login screen.
Because we know all possible provider types and can prepare icons for them.
Only icons that don't fit into the logic now is google-icon.svg and coreos-icon.svg.
Signed-off-by: m.nabokikh maksim.nabokikh@flant.com