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.

Sign up for GitHub

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

fix #7194 Editor errors on Visualize are broken #7702

Merged
merged 6 commits into from
Jul 27, 2016
Merged

fix #7194 Editor errors on Visualize are broken #7702

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c7e89a9
fix #7194 Editor errors on Visualize are broken
ppisljar Jul 11, 2016
be4a47f
- fixing styles to reduce specificity
ppisljar Jul 16, 2016
d019a5e
- adding comment
ppisljar Jul 16, 2016
5a91329
updating to follow new html style guide
ppisljar Jul 26, 2016
7c47e13
updating style
ppisljar Jul 26, 2016
380aafc
fixing typos
ppisljar Jul 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/core_plugins/kibana/public/visualize/editor/sidebar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@
<!-- tabs -->
<ul class="nav navbar-nav">
<li ng-class="{active: sidebar.section == 'data'}" ng-show="sidebar.showData">
<a class="navbar-link active" ng-click="sidebar.section='data'">Data</a>
<a class="vis-editor-subnav-link"
ng-class="{'is-vis-editor-sub-nav-link-selected': sidebar.section == 'data'}"
ng-click="sidebar.section='data'">Data</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit, the attributes should all be on their own lines, the > of the opening tag should be on its own line, and the text and closing tag should be on their own lines:

<a
  class="vis-editor-subnav-link"
  ng-class="{'is-vis-editor-sub-nav-link-selected': sidebar.section == 'data'}"
  ng-click="sidebar.section='data'"
>
  Data
</a>

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm checking some of the other .html files and i cant find this anywhere (last > in a seperate line) ...
also my editor (webstorm) wont put > in line with <a (no indent) if i put it in a seperate line ....
would it be possible to configure .eseditorconfig and .eslint to follow this style ?

also could you update html style guide with this information:

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey Peter, the most recent version of the style guide exemplifies this in the examples. Should I update the descriptions to point this out more, or do you think this is enough?

This is a pretty recent change to the style guide. You can check out this PR for some context.

I'm using Sublime, so I'm not sure why WebStorm would be unhappy with that formatting... is this something you can look into fixing on your end? Let me know if you want to Zoom about this and tackle it together.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks CJ, im not sure how i managed to open the old style guide :) i managed to get webstorm working with it. I updated whole sidebar.html to follow the new style guide, can you please check if it looks ok ?

i still wonder if it would be possible to make a precommit check to enforce this (like we do with eslint) ?

</li>
<li ng-class="{active: sidebar.section == 'options'}">
<a class="navbar-link" ng-click="sidebar.section='options'">Options</a>
<a class="vis-editor-subnav-link"
ng-class="{'is-vis-editor-sub-nav-link-selected': sidebar.section == 'options'}"
ng-click="sidebar.section='options'">Options</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

</li>
</ul>

Expand All @@ -26,7 +30,7 @@
<li ng-if="visualizeEditor.softErrorCount() > 0"
disabled
tooltip="{{ visualizeEditor.describeErrors() }}" tooltip-placement="bottom" tooltip-popup-delay="400" tooltip-append-to-body="1">
<a class="danger navbar-link">
<a class="vis-editor-subnav-link vis-editor-subnav-link--danger">
<i class="fa fa-warning"></i>
</a>
</li>
Expand Down
64 changes: 34 additions & 30 deletions src/core_plugins/kibana/public/visualize/editor/styles/_editor.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,6 @@
.vis-editor {
.flex-parent();

// For the vis-editor sidebar nav
.navbar-default .navbar-nav {
&> .active > a {
border-bottom: 2px solid @kibanaGray2;
color: @kibanaGray1;
&:before {
display: none;
}
&:hover {
background-color: transparent;
}
}

> li {
> a {
padding: 4px 0;
margin: 0 10px;
color: @kibanaGray2;
}
> a:hover {
border-bottom: 2px solid @kibanaGray2;
}
}

.danger {
color: @vis-editor-navbar-error-state-color;
background-color: @vis-editor-navbar-error-state-bg;
}
}

.btn-xs {
line-height: 1.3;
}
Expand Down Expand Up @@ -382,6 +352,40 @@ vis-editor-vis-options > * {
.flex-parent();
}

/**
* 1. TODO: Override bootstrap styles. Remove !important once we're rid of bootstrap.
*/
.vis-editor-subnav-link {
padding: 4px 10px 5px 10px !important; /* 1 */
color: @kibanaGray2 !important; /* 1 */

&.is-vis-editor-sub-nav-link-selected {
border-bottom: 2px solid @kibanaGray2;
color: @kibanaGray1;
background-color: #f6f6f6;

&:before {
display: none;
}

&:hover {
background-color: transparent;
}
}
}

/**
* 1. TODO: Override bootstrap styles. Remove !important once we're rid of bootstrap.
*/
.vis-editor-subnav-link--danger {
color: @vis-editor-navbar-error-state-color !important; /* 1 */
background-color: @vis-editor-navbar-error-state-bg;

&:hover {
background-color: darken(@vis-editor-navbar-error-state-bg, 12%) !important; /* 1 */
}
}

form.vis-share {
div.form-control {
height: inherit;
Expand Down