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 offset override should only apply to icons, not all SVGs #15132

Closed
garrett opened this issue Jan 13, 2021 · 4 comments
Closed

Icon offset override should only apply to icons, not all SVGs #15132

garrett opened this issue Jan 13, 2021 · 4 comments
Assignees

Comments

@garrett
Copy link
Member

garrett commented Jan 13, 2021

We need to class this override:

/* All SVGs used in PF4 have some inline style to align them
* https://github.com/patternfly/patternfly-react/issues/4767
*/
svg {
vertical-align: -0.125em;
}

...As we're now using SVGs that are not icons, such as in the performance page, the SVG graphs, etc.

@garrett
Copy link
Member Author

garrett commented Jan 13, 2021

Ah, PF4 icons are unclassed SVGs, but have the following attributes:

                          height="1em"
                          role="img"
                          width="1em" 

Perhaps we should use svg[height=1em][width=1em][role=img] { vertical-align: -0.125em; } until we get the proper fix from upstream?

@KKoukiou
Copy link
Contributor

There is already a PR upstream for your interest. patternfly/patternfly-react#5275

@garrett
Copy link
Member Author

garrett commented Jan 13, 2021

@KKoukiou: Thanks for linking it here!

It isn't ready in a release we can use yet, so I think we still need a fixed workaround until then. Hopefully it won't have to last so long in our codebase.

@garrett
Copy link
Member Author

garrett commented Jan 13, 2021

As is, vertical-align does get set on all SVGs, but only actually gets applied to inline SVGs or ones in a table cell.

So, basically: it would only affect icons with the incorrect offset anyway. Block SVGs and those inside of a flex or grid (with other specific alignment) actually ignore the vertical-align and are aligned in the flow or as specified (for flex/grid).

@garrett garrett closed this as completed Jan 13, 2021
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 a pull request may close this issue.

3 participants