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 realm icon to AccountItems in organisation switcher, Fixes: #392 #4647

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

Conversation

vaibhavs2
Copy link
Contributor

@vaibhavs2 vaibhavs2 commented Apr 12, 2021

Fixes: #392

1 2 3

The third image above, I created that mannualy to show some realm icons together :)

If some wonder why I made icon container round look at the image below(not looking good), sometime we can get round icon sometime square shaped, so its better to have round container, we can adjust square shaped Icons in round container.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 12, 2021

If some wonder why I made icon container round look at the image below(not looking good), sometime we can get round icon sometime square shaped, so its better to have round container, we can adjust square shaped Icons in round container.

I disagree; I think we'd be better off using the image with no shape change, or at most make it a square with (slightly) rounded corners. I think the basic reason is that some realm logos weren't built to look best as a circle (especially not one with this choice of radius relative to the image's dimensions), and we shouldn't assume they were. See #205 and #3398 where we settled on rounded-corner squares for avatar images for that reason. 🙂

Passing realIcon as email and realm were used to pass so we can have them always and will store in the same way.
@vaibhavs2
Copy link
Contributor Author

Made corner slightly rounded :)

again above list mannualy created

@vaibhavs2
Copy link
Contributor Author

@chrisbobbe thanks for review, now its ready for another review :)

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

Successfully merging this pull request may close these issues.

Use realm icons in organization switcher
2 participants