-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Add registry for doc table details #6105
Changes from 2 commits
0af354f
19022d2
596f350
5a7669c
5d5e8bf
c54fbb1
ec22f09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,47 +1,52 @@ | ||
import _ from 'lodash'; | ||
import angular from 'angular'; | ||
import 'ace'; | ||
import html from 'ui/doc_viewer/doc_viewer.html'; | ||
import $ from 'jquery'; | ||
import 'ui/doc_viewer/doc_viewer.less'; | ||
define(function (require) { | ||
|
||
import 'ui/doc_viewer/viewers/table.js'; | ||
import 'ui/doc_viewer/viewers/json.js'; | ||
|
||
define(function (require) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this file is basically rewritten anyway, mind removing the AMD wrapper? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
require('ui/modules').get('kibana') | ||
.directive('docViewer', function (config, Private) { | ||
const docTableDetailViews = Private(require('ui/registry/doc_table_detail_views')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest following the pattern used in https://github.com/elastic/kibana/pull/6102/files. We will be moving to it very soon There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
return { | ||
restrict: 'E', | ||
template: html, | ||
scope: { | ||
hit: '=', | ||
indexPattern: '=', | ||
filter: '=?', | ||
columns: '=?' | ||
}, | ||
link: { | ||
pre($scope) { | ||
$scope.aceLoaded = (editor) => { | ||
editor.$blockScrolling = Infinity; | ||
}; | ||
}, | ||
|
||
post($scope, $el, attr) { | ||
// If a field isn't in the mapping, use this | ||
$scope.mode = 'table'; | ||
$scope.mapping = $scope.indexPattern.fields.byName; | ||
$scope.flattened = $scope.indexPattern.flattenHit($scope.hit); | ||
$scope.hitJson = angular.toJson($scope.hit, true); | ||
$scope.formatted = $scope.indexPattern.formatHit($scope.hit); | ||
$scope.fields = _.keys($scope.flattened).sort(); | ||
|
||
$scope.toggleColumn = function (fieldName) { | ||
_.toggleInOut($scope.columns, fieldName); | ||
}; | ||
template: function ($el, $attr) { | ||
const $viewer = $('<div class="doc-viewer">'); | ||
$el.append($viewer); | ||
const $tabs = $('<ul class="nav nav-tabs">'); | ||
const $content = $('<div class="doc-viewer-content">'); | ||
$viewer.append($tabs); | ||
$viewer.append($content); | ||
docTableDetailViews.inOrder.forEach(view => { | ||
const $tab = $(`<li ng-show="docViews['${view.title}'].shouldShow(hit)" ng-class="{active: mode == '${view.title}'}"> | ||
<a ng-click="mode='${view.title}'">${view.title}</a> | ||
</li>`); | ||
$tabs.append($tab); | ||
const $ext = $(`<render-directive ng-show="mode == '${view.title}'" definition="docViews['${view.title}']" scope="innerScope"> | ||
</render-directive>`); | ||
$ext.html(view.template); | ||
$content.append($ext); | ||
}); | ||
return $el.html(); | ||
}, | ||
controller: function ($scope) { | ||
$scope.mode = docTableDetailViews.inOrder[0].title; | ||
$scope.docViews = docTableDetailViews.byTitle; | ||
|
||
$scope.showArrayInObjectsWarning = function (row, field) { | ||
var value = $scope.flattened[field]; | ||
return _.isArray(value) && typeof value[0] === 'object'; | ||
}; | ||
} | ||
$scope.innerScope = { | ||
hit: $scope.hit, | ||
indexPattern: $scope.indexPattern, | ||
filter: $scope.filter, | ||
columns: $scope.columns | ||
}; | ||
} | ||
}; | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<div | ||
id="json-ace" | ||
ng-model="hitJson" | ||
readonly | ||
ui-ace="{ | ||
useWrapMode: true, | ||
onLoad: aceLoaded, | ||
advanced: { | ||
highlightActiveLine: false | ||
}, | ||
rendererOptions: { | ||
showPrintMargin: false, | ||
maxLines: 4294967296 | ||
}, | ||
mode: 'json' | ||
}"></div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import _ from 'lodash'; | ||
import angular from 'angular'; | ||
import 'ace'; | ||
import docTableDetailViewsRegistry from 'ui/registry/doc_table_detail_views'; | ||
|
||
import jsonHtml from './json.html'; | ||
|
||
docTableDetailViewsRegistry.register(function () { | ||
return { | ||
title: 'JSON', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer if these objects didn't mix together the properties needed for the return {
title: ...
order: ...
directive: {
template: ...,
controller() { }
}
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
order: 20, | ||
template: jsonHtml, | ||
shouldShow: () => true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally any doc view would be able to render any hit, so if you don't have a solid use case for Otherwise you probably want to make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a few doc views that I only want to show for certain hits. It looks at fields in the hit to determine if the doc view is useful or not (will it show something or will it just be a blank view). That's why I added the shouldShow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, works for me. I do think the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed |
||
controller: function ($scope) { | ||
$scope.indexPattern = $scope.scope.indexPattern; | ||
$scope.hit = $scope.scope.hit; | ||
$scope.columns = $scope.scope.columns; | ||
$scope.filter = $scope.scope.filter; | ||
|
||
$scope.hitJson = angular.toJson($scope.hit, true); | ||
|
||
$scope.aceLoaded = (editor) => { | ||
editor.$blockScrolling = Infinity; | ||
}; | ||
} | ||
}; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
<table class="table table-condensed"> | ||
<tbody> | ||
<tr ng-repeat="field in fields"> | ||
<td field-name="field" | ||
field-type="mapping[field].type" | ||
width="1%" | ||
class="doc-viewer-field"> | ||
</td> | ||
<td width="1%" class="doc-viewer-buttons" ng-if="filter"> | ||
<span ng-if="mapping[field].filterable"> | ||
<i ng-click="filter(mapping[field], flattened[field], '+')" | ||
tooltip="Filter for value" | ||
tooltip-append-to-body="1" | ||
class="fa fa-search-plus"></i> | ||
<i ng-click="filter(mapping[field], flattened[field],'-')" | ||
tooltip="Filter out value" | ||
tooltip-append-to-body="1" | ||
class="fa fa-search-minus"></i> | ||
</span> | ||
<span ng-if="!mapping[field].filterable" tooltip="Unindexed fields can not be searched"> | ||
<i class="fa fa-search-plus text-muted"></i> | ||
<i class="fa fa-search-minus text-muted"></i> | ||
</span> | ||
<span ng-if="columns"> | ||
<i ng-click="toggleColumn(field)" | ||
tooltip="Toggle column in table" | ||
tooltip-append-to-body="1" | ||
class="fa fa-columns"></i> | ||
</span> | ||
</td> | ||
|
||
<td> | ||
<i ng-if="!mapping[field] && field[0] === '_'" | ||
tooltip-placement="top" | ||
tooltip="Field names beginning with _ are not supported" | ||
class="fa fa-warning text-color-warning ng-scope doc-viewer-underscore"></i> | ||
<i ng-if="!mapping[field] && field[0] !== '_' && !showArrayInObjectsWarning(doc, field)" | ||
tooltip-placement="top" | ||
tooltip="No cached mapping for this field. Refresh field list from the Settings > Indices page" | ||
class="fa fa-warning text-color-warning ng-scope doc-viewer-no-mapping"></i> | ||
<i ng-if="showArrayInObjectsWarning(doc, field)" | ||
tooltip-placement="top" | ||
tooltip="Objects in arrays are not well supported." | ||
class="fa fa-warning text-color-warning ng-scope doc-viewer-object-array"></i> | ||
<div class="doc-viewer-value" ng-bind-html="typeof(formatted[field]) === 'undefined' ? hit[field] : formatted[field] | trustAsHtml"></div> | ||
</td> | ||
</tr> | ||
</tbody> | ||
</table> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import _ from 'lodash'; | ||
import docTableDetailViewsRegistry from 'ui/registry/doc_table_detail_views'; | ||
|
||
import tableHtml from './table.html'; | ||
|
||
docTableDetailViewsRegistry.register(function () { | ||
return { | ||
title: 'Table', | ||
order: 10, | ||
template: tableHtml, | ||
shouldShow: () => true, | ||
controller: function ($scope) { | ||
$scope.indexPattern = $scope.scope.indexPattern; | ||
$scope.hit = $scope.scope.hit; | ||
$scope.columns = $scope.scope.columns; | ||
$scope.filter = $scope.scope.filter; | ||
|
||
$scope.mapping = $scope.indexPattern.fields.byName; | ||
$scope.flattened = $scope.indexPattern.flattenHit($scope.hit); | ||
$scope.formatted = $scope.indexPattern.formatHit($scope.hit); | ||
$scope.fields = _.keys($scope.flattened).sort(); | ||
|
||
$scope.toggleColumn = function (fieldName) { | ||
_.toggleInOut($scope.columns, fieldName); | ||
}; | ||
|
||
$scope.showArrayInObjectsWarning = function (row, field) { | ||
var value = $scope.flattened[field]; | ||
return _.isArray(value) && typeof value[0] === 'object'; | ||
}; | ||
} | ||
}; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
define(function (require) { | ||
return require('ui/registry/_registry')({ | ||
name: 'docTableDetailViews', | ||
index: ['title'], | ||
order: ['order'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to implement the defaults I mentioned above you will either want to create a return require('ui/registry/_registry')({
name: 'docTableDetailViews',
constructor() {
this.forEach(docView => {
docView.shouldShow = docView.shouldShow || _.constant(true)
})
}
}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went with the constructor approach. I also added a new field called "name". It defaults to title but is meant for the case where title has quotes or other strange characters that might mess up the docViews[title] code. So, now you can specify a name in that case. |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,8 @@ module.directive('renderDirective', function () { | |
return { | ||
restrict: 'E', | ||
scope: { | ||
'definition': '=' | ||
'definition': '=', | ||
'scope': '=?' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😔 not sure how we can make this better but I hope we can come up with something. Referring to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the definition included a scope definition? We would have to implement that in render_directive, but that sounds like fun to me. definition = {
controller...
controllerAs...
scope: {
hit: '=',
indexPattern: '=',
onSave: '&'
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took a stab at this in #6115, mind trying it out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried it out. It works but feels a little awkward. I have to define the attributes in both the definition and in the render-directive html. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated the pull request and you can see how I used it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I understand how that might be a bit awkward but I see it as documenting what you pass in and what you expect to be passed in. |
||
}, | ||
template: function ($el, $attrs) { | ||
return $el.html(); | ||
|
@@ -27,4 +28,4 @@ module.directive('renderDirective', function () { | |
} | ||
} | ||
}; | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this as an extension point for plugins, but I think the existing doc viewers should use it (rather than being included directly). Perhaps they should get their own plugin, similar to the
kbn_vislib_vis_types
plugin.Since there isn't any direct dependence on the
docTable
It may make sense to go generic with this export type and just call itdocViews
, then thedocViewer
(or any other component that wants to display docs) can utilize them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created kbn_doc_views plugin and moved them there. The registry is now named docViews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃