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

Fix FontAwesome icon width in navbar app icons #3075

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

robinkar
Copy link
Contributor

Currently some of the FontAwesome icons flow over the fixed width that is set for the icons. Visible in the screenshot here:
iconissue

This PR sets the proper width for FA icons, while keeping the size the same:
iconissue_fixed

This happens since FA sets the width of .fa-fw to 1.25em. I added margin-right to non-FA icons to align app names properly, although another option would be to set width (and height) to 1.25em for those, but that would affect the size or aspect ratio of them.

FontAwesome defines .fa-fw to have width 1.25em. With the previous 1/1
ratio, the icon flows over to the text to the right. Use margin-right for
other icons to align.
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

Thanks. Tests are flaky for some reason, but this looks good.

@johrstrom johrstrom self-requested a review September 27, 2023 13:44
@johrstrom
Copy link
Contributor

Sorry as soon as I saw your images I recognized that 14px.

Why not modify this?

.navbar .app-icon {
width: 14px;
height: 14px;
font-size: 14px;
}

@robinkar
Copy link
Contributor Author

Sorry as soon as I saw your images I recognized that 14px.

Why not modify this?

.navbar .app-icon {
width: 14px;
height: 14px;
font-size: 14px;
}

Modifying that affects the non-FA images too. Changing the width there makes the other app icons stretched out, although it is not too noticeable since they are so small.

Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

Hmmmm.... OK we can go with this. I could have sworn someone was complaining about that 14px (being too small?) - but I cannot now find the discourse topic or issue for the same.

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.

3 participants