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

fixed icon missing issue #4411

Merged
merged 1 commit into from
Jun 1, 2024
Merged

fixed icon missing issue #4411

merged 1 commit into from
Jun 1, 2024

Conversation

ibulmer
Copy link
Collaborator

@ibulmer ibulmer commented May 31, 2024

Description

Fix missing icon issue.

Screenshots

Before
image

After
image

@cielf
Copy link
Collaborator

cielf commented May 31, 2024

Hey @ibulmer -- thanks for catching this! Some thoughts;
1/ for consistency, I would expect the icon foreground to be white.
2/ Rather a A lot of tests failed - are they failing on your local? It looks to me like they are related to the added gem.
3/ It looks like you're adding a new gem just to add this icon -- that might be overkill. Is there something in fontawesome (which I believe is where the other icons come from) that might give the idea of organizations? (Edit: maybe institution? )

@ibulmer
Copy link
Collaborator Author

ibulmer commented May 31, 2024

Hey @ibulmer -- thanks for catching this! Some thoughts; 1/ for consistency, I would expect the icon foreground to be white. 2/ Rather a A lot of tests failed - are they failing on your local? It looks to me like they are related to the added gem. 3/ It looks like you're adding a new gem just to add this icon -- that might be overkill. Is there something in fontawesome (which I believe is where the other icons come from) that might give the idea of organizations? (Edit: maybe institution? )

Hey @cielf good idea! Have updated the pr.

@scooter-dangle
Copy link
Collaborator

Thanks!

@scooter-dangle scooter-dangle merged commit 9e14566 into main Jun 1, 2024
38 checks passed
@scooter-dangle scooter-dangle deleted the icons branch June 1, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants