Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Aligned the bookmark icon on the navbar #5739

Merged
merged 1 commit into from
Nov 29, 2016
Merged

Aligned the bookmark icon on the navbar #5739

merged 1 commit into from
Nov 29, 2016

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Nov 19, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

This PR addresses the issue which was caused by introducing SVG toolbar icons and placing them with pixels, by removing margins around the bookmark icon and setting background-size.

Fixes #5632

Auditors: @bradleyrichter

Test Plan: make sure the bookmark icon is properly aligned on each platforms

Image 1: on Ubuntu
pasted image at 2016_11_03 01_37 pm

Image 2: on macOS
screenshot 2016-11-19 21 12 26

Related: #5173 #4852

@luixxiul luixxiul added this to the 0.12.11 milestone Nov 19, 2016
@luixxiul
Copy link
Contributor Author

luixxiul commented Nov 19, 2016

This depends on #5750. Please do not merge this before #5750 is merged.

width: 15px;
height: 15px;
position: relative;
left: 5px;
Copy link
Contributor Author

@luixxiul luixxiul Nov 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 5px is a magic number. At first I tried display: flex, justify-content: center, but it did not work well with box-sizing: border-box on macOS, while it did on Ubuntu.

@bbondy
Copy link
Member

bbondy commented Nov 23, 2016

This is not working right on macOS:

screenshot 2016-11-22 22 56 47

@bradleyrichter
Copy link
Contributor

bradleyrichter commented Nov 23, 2016

or in master:

image

image

@luixxiul
Copy link
Contributor Author

Conflicted. I will rebase.

Fixes #5632

Auditors: @bradleyrichter

Test Plan: make sure the bookmark icon is properly aligned on each platforms
@luixxiul
Copy link
Contributor Author

Updated. Please review, thanks.

@bbondy
Copy link
Member

bbondy commented Nov 29, 2016

++

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

Successfully merging this pull request may close these issues.

3 participants