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

settings icons fix #11572

Merged
merged 4 commits into from Apr 17, 2018

Conversation

Projects
None yet
3 participants
@Ijin08
Contributor

Ijin08 commented Apr 12, 2018

added styling to fontawesome icons so they have same size as the other icons

skarmavbild 2018-04-12 kl 10 42 38

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Apr 12, 2018

Codecov Report

Merging #11572 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #11572   +/-   ##
=======================================
  Coverage    51.9%    51.9%           
=======================================
  Files         359      359           
  Lines       26068    26068           
  Branches     1571     1571           
=======================================
  Hits        13530    13530           
  Misses      11796    11796           
  Partials      742      742

codecov-io commented Apr 12, 2018

Codecov Report

Merging #11572 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #11572   +/-   ##
=======================================
  Coverage    51.9%    51.9%           
=======================================
  Files         359      359           
  Lines       26068    26068           
  Branches     1571     1571           
=======================================
  Hits        13530    13530           
  Misses      11796    11796           
  Partials      742      742
@marefr

Code looks good. Seem to be some inconsistencies regarding height of menu item/icon positioning. Seems like the permissions link is the only one higher than the others. Personally to me it looks like the versions and permissions icons not are aligned vertically with text. See screenshots below.

image

image

@Ijin08

This comment has been minimized.

Show comment
Hide comment
@Ijin08

Ijin08 Apr 16, 2018

Contributor

@marefr I know, the problem here is the fontawesome icon that has white space beneath it so it gets pushed up. I am not sure how to push it down.

Contributor

Ijin08 commented Apr 16, 2018

@marefr I know, the problem here is the fontawesome icon that has white space beneath it so it gets pushed up. I am not sure how to push it down.

@marefr

This comment has been minimized.

Show comment
Hide comment
@marefr

marefr Apr 16, 2018

Member

Tried to make it a little better:
image

@bulletfactory @Ijin08 what do you think?

Member

marefr commented Apr 16, 2018

Tried to make it a little better:
image

@bulletfactory @Ijin08 what do you think?

marefr added some commits Apr 16, 2018

@marefr

marefr approved these changes Apr 16, 2018

@marefr marefr merged commit d9799f7 into master Apr 17, 2018

4 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test-backend Your tests passed on CircleCI!
Details
ci/circleci: test-frontend Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@marefr marefr deleted the settings-icons-fix branch Apr 17, 2018

@marefr marefr added this to the 5.1 milestone Apr 17, 2018

marefr added a commit that referenced this pull request Apr 17, 2018

ryantxu added a commit to NatelEnergy/grafana that referenced this pull request Apr 18, 2018

Merge remote-tracking branch 'grafana/master'
* grafana/master: (52 commits)
  changelog: adds note for #11173
  changelog: fix typo
  changelog: notes about closing #11572
  Fix issues with metric reporting (#11518)
  changelog: notes about closing #10747
  fix: Row state is now ignored when looking for dashboard changes (#11608)
  disable codecov comments
  add some more sort order asserts for permissions store tests
  Revert "build: remove code cov"
  Revert "removes codecov from frontend tests"
  tsdb: update query and annotation editor help texts for postgres
  changelog: notes about closing #11578
  calculate datetime for timeFrom and timeTo macro in go
  set default for sslmode to verify-full in postgres datasource editor (#11615)
  add some more sort order asserts for permissions store tests
  fix unconvert issues
  variable: fix binding bug after ts conversion
  add GetFromAsTimeUTC and GetToAsTimeUTC and use them in timeFilter macro
  fix merge conflict
  remove changes to module.ts from this branch
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment