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

Nav bar tweaks #6157

Closed
wants to merge 22 commits into from
Closed

Conversation

rashidkpc
Copy link
Contributor

screen shot 2016-02-08 at 5 10 15 pm

  • Implements new icons for Discover/Visualize/Dashboard
  • Brings in @spalger's pull for plugins exporting links
  • Changed the logo svg to be inkscape compatible and added some padding
  • Added the drop shadow from the app container
  • Added a hover when mousing over app
  • Vertically centered icons, left justified links
  • Reduced width of sidebar to 160px

@rashidkpc
Copy link
Contributor Author

This has a bug, the app-wrapper doesn't actually wrap the entire app and ends at the height of the viewport

screen shot 2016-02-08 at 5 01 32 pm

@alt74
Copy link

alt74 commented Feb 9, 2016

could we increase vertical spacing between icons? example:
k5-nav

@@ -9,8 +9,8 @@
ng-href="{{ link.active ? link.url : (link.lastSubUrl || link.url) }}"
data-test-subj="appLink">

<div ng-if="link.icon" ng-style="{ 'background-image': 'url(../' + link.icon + ')' }" class="app-icon"></div>
<div ng-if="!link.icon" class="app-icon app-icon-missing">{{ link.title[0] }}</div>
<div ng-if="link.icon" ng-style="{ 'mask-image': 'url(../' + link.icon + ')' }" class="app-icon"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Weren't we going to use just an img tag for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no strong opinion on this. From a user standpoint it works the same. @simianhacker any reason you went this direction?

@rashidkpc
Copy link
Contributor Author

jenkins, test it

@panda01 panda01 mentioned this pull request Feb 9, 2016
@panda01
Copy link
Contributor

panda01 commented Feb 9, 2016

Continuing this in the pr above.

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

Successfully merging this pull request may close these issues.

6 participants