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

Improve scaling and vertical alignment of browse icons #5354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomhughes
Copy link
Member

Although we're using SVG icons because they are being added using pseudo elements it's currently impossible to directly set a size for them to scale to as https://stackoverflow.com/a/8993934 explains.

So instead we use a solution based on the alternate approach there of setting the image as a background image, which can be scaled to our preferred size and then aligned.

The only tricky part is working out the vertical size to scale to - just using 1em doesn't really look right as it doesn't allow for the descenders so I've scaled it up a bit which seems to look about right in firefox and chrome.

Here's a before and after example:

image

Although we're using SVG icons because they are being added using
pseudo elements it's currently impossible to directly set a size for
them to scale to as https://stackoverflow.com/a/8993934 explains.

So instead we use a solution based on the alternate approach there
of setting the image as a background image, which can be scaled to
our preferred size and then aligned.
@AntonKhorev
Copy link
Collaborator

vertical size to scale

Why do you need to scale the icons?

they are being added using pseudo elements

That's what I was going to stop doing after #5317. I was going to write the icons as <img> or <svg>, set their size to 20x20 px, center them inside that size using object-fit: none (this lets the actual image to be smaller than 20x20, which is usually the case) and put them in display: flex element (possible after the inline icons inside "part of ways" are removed), they align fine with the text this way.

@tomhughes
Copy link
Member Author

Well it mostly started out as me trying to fix the alignment and the scaling became a kind of side effect as I realised that thing weren't very consistently sized. Possibly now that I know about the background image hack that might not be strictly necessary...

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 this pull request may close these issues.

2 participants