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

Fixes #9798: UI to display tags #1421

Conversation

RaphaelGauthier
Copy link
Member

@RaphaelGauthier
Copy link
Member Author

Commit modified

@@ -640,6 +716,8 @@ class RuleGrid(
val t5 = System.currentTimeMillis
TimingDebugLogger.trace(s"Rule grid: transforming into data: get rule data: callback: ${t5-t4}ms")

val tags = """[{"key":"environment","value":"production"},{"key":"customer","value":"Acme prod"}]"""
Copy link
Member

Choose a reason for hiding this comment

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

You should use the tag stored in the Rule object

@@ -218,7 +218,7 @@ object DisplayDirectiveTree extends Loggable {
}
val htmlId = s"jsTree-${directive.id.value}"
override val attrs = (
("data-jstree" -> """{ "type" : "directive" }""") ::
Copy link
Member

Choose a reason for hiding this comment

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

You should use the Tags stored in the directive object

@@ -385,6 +393,7 @@ function createRuleTable(gridId, data, needCheckbox, needActions, needCompliance

// Choose which columns should be included
var columns = [];
columns.push(tags);
Copy link
Member

Choose a reason for hiding this comment

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

If you had pushed them at the end you would'nt have to change the filter column ;) but since it's done, it's ok!!

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 will remember it ;)

@@ -523,6 +539,7 @@ function createRuleComplianceTable(gridId, data, contextPath, refresh) {
, "oLanguage": {
"sSearch": ""
}
, "columnDefs": [{ "visible": false, "targets": 0 }]
Copy link
Member

Choose a reason for hiding this comment

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

It's in reporting table, is it hiding the first column ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I think I just forgot to delete this line!

</style>
<link media="screen" href="/style/rudder/rudder-tags.css" rel="stylesheet" type="text/css">
<script type="text/javascript" src="/javascript/rudder/angular/filters.js"></script>
<script>
Copy link
Member

Choose a reason for hiding this comment

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

Can you put it in a file (directive.js or directive-filter.js)

@VinceMacBuche
Copy link
Member

Apart the small comments it seems ok !

@@ -0,0 +1,51 @@
var app = angular.module('tags', []);
var xxxx;
Copy link
Member

Choose a reason for hiding this comment

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

This var should be remmoved, or renamed :)

@RaphaelGauthier
Copy link
Member Author

Commit modified

@RaphaelGauthier
Copy link
Member Author

PR rebased

@RaphaelGauthier
Copy link
Member Author

Commit modified

@@ -165,6 +165,7 @@ class RuleGrid(
def asyncDisplayAllRules(onlyRules: Option[Set[RuleId]], showComplianceColumns: Boolean, showRecentChanges: Boolean) = {
AnonFunc(SHtml.ajaxCall(JsNull, (s) => {

val defaultTag = Tags(Set(Tag(TagName("env"),TagValue("prod"))))
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the default tags ?

"""
<div id={htmlId_rulesGridZone}>
<span class="error" id="ruleTableError">{errorProperty.getOrElse("")}</span>
<div id={htmlId_modalReportsPopup} class="nodisplay">
<div id={htmlId_reportsPopup} ></div>
</div>
<div id="showFiltersRules" ng-controller="filterTagRuleCtrl" class="filters tw-bs" ng-cloak="">
Copy link
Member

Choose a reason for hiding this comment

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

Can you put all of this in an html file and include it here ?


var listTags = tags
def cfTagsConfiguration(controllerID:String) =
<div class="row form-group" ng-controller={{controllerID}} ng-cloak="" >
Copy link
Member

Choose a reason for hiding this comment

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

Would be better if html was in an html file and only modifying the ng-controller attribute using lift css selector (like we do for directive inputs"

@RaphaelGauthier
Copy link
Member Author

Commit modified

}

// PLUGIN JSTREE : SEARCH-TAG
$.jstree.plugins.searchtag = function (options, parent) {
Copy link
Member

Choose a reason for hiding this comment

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

That Plugin should be in a separate file

Copy link
Member

Choose a reason for hiding this comment

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

Added into common layout before tree.js

@@ -0,0 +1,533 @@
var app = angular.module('filters', []);
Copy link
Member

Choose a reason for hiding this comment

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

You need to add the license to the top of the fule

@@ -0,0 +1,68 @@
var app = angular.module('tags', []);
Copy link
Member

Choose a reason for hiding this comment

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

Need license too

app.controller('tagsRuleCtrl', function ($scope, $http, $location, $timeout) {
$scope.scopeFilter;

$scope.init = function(treeId, tags){
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need treeid ? ( I saw it as a constant )

}
var temp;
for (key in jsonTags){
temp = {'key':key, 'value':jsonTags[key]};
Copy link
Member

Choose a reason for hiding this comment

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

Temp should be declared here

@@ -0,0 +1,15 @@
$(document).ready(function(){
Copy link
Member

Choose a reason for hiding this comment

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

Need license

@VinceMacBuche
Copy link
Member

We will treat those remarks in a new PR

@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 9f0d717 into Normation:branches/rudder/4.1 Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants