Add registry for doc table details #6105

trevan · 2016-02-04T07:03:02Z

This PR creates a registry for doc table details. It moves the table and json viewer into the registry. You can control whether a viewer should be shown for a specific hit.

It uses the render_directive but I couldn't figure out how to pass scope variables into it so I modified the directive and added a "scope" attribute. If there is a better way, I'll change it.

I'm working on the tests but wanted to get the pull request submitted for feedback.

This partially closes #5214

elasticsearch-release · 2016-02-04T07:03:04Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

…gistry Conflicts: src/plugins/kibana/index.js src/ui/ui_exports.js

jimmyjones2 · 2016-02-04T08:08:40Z

Just what I need, big +1, thanks!

spalger · 2016-02-04T13:28:37Z

This is awesome, excited to review

spalger · 2016-02-04T13:36:06Z

src/ui/public/render_directive/render_directive.js

@@ -6,7 +6,8 @@ module.directive('renderDirective', function () {
  return {
    restrict: 'E',
    scope: {
-      'definition': '='
+      'definition': '=',
+      'scope': '=?'


😔 not sure how we can make this better but I hope we can come up with something. Referring to $scope.scope internally is gross.

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: '&' } }

I took a stab at this in #6115, mind trying it out?

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.

I've updated the pull request and you can see how I used it.

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.

People can use the `<render-directive>` directive to programatically extend views, but passing values to those views is not as easy. To make it easier we will support defining scope bindings just like directives support, and those bindings will be applied to the isolate scope created for the rendered directive. The `<render-directive>` the majority of angular 1.4s syntax. The differences include: 1. `=?` is not implemented 1. Isolate scopes are always created, even is `scope:` is not defined

spalger · 2016-02-04T15:56:31Z

src/plugins/kibana/index.js

@@ -22,7 +22,8 @@ module.exports = function (kibana) {
          'spyModes',
          'fieldFormats',
          'navbarExtensions',
-          'settingsSections'
+          'settingsSections',
+          'docTableDetailViews'


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 it docViews, then the docViewer (or any other component that wants to display docs) can utilize them.

I created kbn_doc_views plugin and moved them there. The registry is now named docViews.

spalger · 2016-02-04T16:14:09Z

jenkins, test it

…indings' into doc-detail-views-registry

Use the new render-directive attr mechanism. Rename the registry to be more generic Create default values Add a name field in case the title has quotes or other characters Update import code style

trevan · 2016-02-04T19:37:50Z

@spalger, I think I've resolved your comments. I also fixed up the tests so they should pass now.

epixa · 2016-02-04T20:20:22Z

jenkins, test it

trevan · 2016-02-04T21:25:04Z

@epixa, I don't understand why the build failed. It has the message:

/home/jenkins/workspace/kibana_core_pr/ephemeral-x-f5309ec.sh: line 2: syntax error near unexpected token newline' /home/jenkins/workspace/kibana_core_pr/ephemeral-x-f5309ec.sh: line 2:'

Any ideas?

epixa · 2016-02-04T21:33:36Z

Hmmm... I actually have no idea off-hand, but it certainly doesn't seem like anything that has to do with your PR.

epixa · 2016-02-04T21:33:40Z

jenkins, test it

epixa · 2016-02-04T21:48:50Z

That one seemed to work. ¯_(ツ)_/¯

trevan · 2016-02-05T16:17:16Z

@spalger, @epixa what is left for me to do?

spalger · 2016-02-05T22:05:16Z

Haha, nothing!

Add registry for doc table details

spalger · 2016-02-05T22:05:32Z

Very nice @trevan!

spalger · 2016-02-05T22:06:45Z

trevan · 2016-02-05T22:10:58Z

@spalger, thanks

epixa · 2016-02-05T22:37:40Z

@spalger Can you think of any reason why this couldn't go into 4.5?

Add registry for doc table details

0af354f

trevan mentioned this pull request Feb 4, 2016

Formatting plugins for JSON results #5214

Closed

Merge remote-tracking branch 'kibana/master' into doc-detail-views-re…

19022d2

…gistry Conflicts: src/plugins/kibana/index.js src/ui/ui_exports.js

spalger self-assigned this Feb 4, 2016

spalger reviewed Feb 4, 2016
View reviewed changes

Trevan Richins added 4 commits February 4, 2016 10:48

Merge remote-tracking branch 'spalger/implement/renderDirectiveScopeB…

5a7669c

…indings' into doc-detail-views-registry

Resolve review comments

5d5e8bf

Use the new render-directive attr mechanism. Rename the registry to be more generic Create default values Add a name field in case the title has quotes or other characters Update import code style

Move doc viewers to plugin

c54fbb1

Fix tests for doc_views

ec22f09

spalger added a commit that referenced this pull request Feb 5, 2016

Merge pull request #6105 from trevan/doc-detail-views-registry

0b122eb

Add registry for doc table details

spalger merged commit 0b122eb into elastic:master Feb 5, 2016

trevan deleted the doc-detail-views-registry branch February 5, 2016 22:10

epixa added review v5.0.0 labels Feb 5, 2016

tbragin mentioned this pull request Mar 18, 2016

Kibana 5.0.0-alpha1 docs / release notes #6572

Closed

7 tasks

epixa added v5.0.0-alpha1 and removed v5.0.0-alpha1 labels Mar 31, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add registry for doc table details #6105

Add registry for doc table details #6105

trevan commented Feb 4, 2016

elasticsearch-release commented Feb 4, 2016

jimmyjones2 commented Feb 4, 2016

spalger commented Feb 4, 2016

spalger Feb 4, 2016

spalger Feb 4, 2016

spalger Feb 4, 2016

trevan Feb 4, 2016

trevan Feb 4, 2016

spalger Feb 4, 2016

spalger Feb 4, 2016

trevan Feb 4, 2016

spalger Feb 4, 2016

spalger commented Feb 4, 2016

trevan commented Feb 4, 2016

epixa commented Feb 4, 2016

trevan commented Feb 4, 2016

epixa commented Feb 4, 2016

epixa commented Feb 4, 2016

epixa commented Feb 4, 2016

trevan commented Feb 5, 2016

spalger commented Feb 5, 2016

spalger commented Feb 5, 2016

spalger commented Feb 5, 2016

trevan commented Feb 5, 2016

epixa commented Feb 5, 2016

Add registry for doc table details #6105

Add registry for doc table details #6105

Conversation

trevan commented Feb 4, 2016

elasticsearch-release commented Feb 4, 2016

jimmyjones2 commented Feb 4, 2016

spalger commented Feb 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger commented Feb 4, 2016

trevan commented Feb 4, 2016

epixa commented Feb 4, 2016

trevan commented Feb 4, 2016

epixa commented Feb 4, 2016

epixa commented Feb 4, 2016

epixa commented Feb 4, 2016

trevan commented Feb 5, 2016

spalger commented Feb 5, 2016

spalger commented Feb 5, 2016

spalger commented Feb 5, 2016

trevan commented Feb 5, 2016

epixa commented Feb 5, 2016