Financial-Times / o-icons

Icons for FT sites. http://registry.origami.ft.com/components/o-icons
4 stars 4 forks source link

vertical alignment — padding #58

Closed gvonkoss closed 5 years ago

gvonkoss commented 7 years ago

https://github.com/Financial-Times/o-icons/blob/fce1b73276a0811742559f9d92817a297f12cdea/scss/_mixins.scss#L70 seems to have been a solution for a time when the icons didn't all have padding, and did need to be aligned to the baseline, whereas now it seems like vertical-align: middle; would help avoid overrides in products that use this component.

JakeChampion commented 7 years ago

When trying to align an icon with some text @gvonkoss and myself found that vertical-align: middle; achieves this. We decided to override this inside of o-expander, o-subs-card and Next Article as well.

Making this change inside of o-icons would probably be a breaking change because some consumers would have solved the alignment issue by using absolute positioning and some magic numbers for margin and padding.

@alicebartlett, do you have any thoughts on this at all?

alicebartlett commented 7 years ago

Yes it would be breaking. We should line this up for the next major (which is currently not planned)

notlee commented 5 years ago

The default has been set to middle here https://github.com/Financial-Times/o-icons/pull/89. To be released as part of the major cascade.