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

Icon positioning is super-hacky #31

Open
schluchter opened this issue Mar 31, 2017 · 4 comments
Open

Icon positioning is super-hacky #31

schluchter opened this issue Mar 31, 2017 · 4 comments

Comments

@schluchter
Copy link
Contributor

In every component that uses icons, implementers use different strategies to make them align the way they want to. Because of the way the icon CSS is structured, it's really hard to predict how icons are going to behave when they are placed at different sizes.

Let's clean that up.

@fragsalat
Copy link
Contributor

@schluchter I fixed a small bug of this while doing the demonstration page. The problem was that we used i or span elements to embed icons and those are inline and not inline-block elements. So the width and height are not fixed or predictable. So I changed the elements to be inline-block and removed the workaround with negative text indents to align the icon inside the 16x16 box. This seem to work great except that the icons have these white space around and they are not exactly 16px. But this is something you should already know. :)

@fokusferit
Copy link
Contributor

@fragsalat does it mean this task is already "done " ? Because it is still in Development phase :D

@schluchter schluchter self-assigned this May 30, 2017
@schluchter
Copy link
Contributor Author

Create a number of sample icons without generic whitespace and upload to icon font.

fragsalat added a commit that referenced this issue Jul 27, 2017
Updated color variable values and names
@schluchter
Copy link
Contributor Author

@fokusferit The sample icons are in the icon font. @fragsalat created a demo page:

We need to revisit this issue when the final font decision has been made so we can test the alignment with fonts that don't have the same baseline issue that Acumin Pro does.

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

No branches or pull requests

3 participants