Skip to content

Conversation

@siddharthkp
Copy link
Contributor

@siddharthkp siddharthkp commented Mar 13, 2020

Discussed this one with Danny and realised that our icons can't be on the 4pt scale

Example:

Screenshot 2020-03-13 at 11 21 36 AM

The button that contains both these icons is the same size - 26x26, but the icons inside them have to be adjusted based how they look next to each other

Because of that, they can't always be 16. or even be on the 8/12/16 scale we have. We need more granular control of them

The ones in the image are 14x14 and 10x10 to look optically aligned. (both inside a 26x26 button)

  • Changed the icon component
  • updated all the components that use icon (backward compat lol)

Notes for future: It would be cool to optically align icons in the library so that there are less scenarios in which we need to optically align them. This can't be abstracted away fully because optical alignment depends on the context/neighbouring icons, so we'll always need this escape hatch 🙃

@siddharthkp siddharthkp requested a review from SaraVieira March 13, 2020 10:25
@siddharthkp siddharthkp added the UI label Mar 13, 2020
@lbogdan lbogdan temporarily deployed to pr3689 March 13, 2020 10:31 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Mar 13, 2020

Build for latest commit 972dde4 is at https://pr3689.build.csb.dev/s/new.

@lbogdan lbogdan temporarily deployed to pr3689 March 13, 2020 10:40 Inactive
@SaraVieira SaraVieira merged commit 05f3ec0 into master Mar 17, 2020
@SaraVieira SaraVieira deleted the fix-icon-sizes branch March 17, 2020 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants