Allow other apps to list their custom visualizations in the Visualize app#43386
Allow other apps to list their custom visualizations in the Visualize app#43386chrisdavies merged 14 commits intoelastic:masterfrom
Conversation
that are managed in the Visualize app list.
💔 Build Failed |
| } | ||
|
|
||
| .visListingTable__actionIcon { | ||
| filter: grayscale(100%); |
There was a problem hiding this comment.
This should already be applied if the column is identified as an ActionsColumn (er, has action content). I tried winding my way through the files to figure out why this isn't just working, but couldn't track down the root of the issue. Perhaps I'm missing something, but it seems like something is off here.
There was a problem hiding this comment.
It's because we're using a custom column here, not the standard action, since we want to enable / disable on a per item basis. The selector used by EUI is really specific and involves a 3 class-name chain which doesn't exist in our table here. :/
There was a problem hiding this comment.
I commented on that in the other PR (we have to get into master, this is getting confusing) - as there are only two consumers of the table list view, we can do this in a clean way, solve it in table list view and don't need this custom column
There was a problem hiding this comment.
We'll still need a column like this, however, so we'll still end up with our own CSS, I think for the reason I outlined.
table_list_view, rather than just a visualize override
💔 Build Failed |
|
Please merge master to fix percy failure |
💔 Build Failed |
💔 Build Failed |
| @@ -0,0 +1,12 @@ | |||
|
|
|||
| .kbnTableListView__actionIcon { | |||
| filter: grayscale(100%); | |||
There was a problem hiding this comment.
https://caniuse.com/#search=grayscale
Did you test this to make sure it worked?
There was a problem hiding this comment.
It's a minor thing, so it doesn't need to work perfectly across browsers. I'm just trying to get the table to match the look / feel of the default EUI table which itself uses this filter.
The filter works in FF, Chrome, and Safari, though. Haven't tested in IE or Edge.
There was a problem hiding this comment.
Ideally EuiInMemoryTable would support to have different actions on different rows. I suggest we add a TODO here and open an issue in EUI to add this functionality - then we can clean it up here. What do you think @snide @chrisdavies ?
💔 Build Failed |
| }), | ||
|
|
||
| render: (_field, record) => ( | ||
| <EuiButtonIcon |
There was a problem hiding this comment.
tooltip is missing here - the eui implementation is here: https://github.com/elastic/eui/blob/master/src/components/basic_table/default_item_action.js
flash1293
left a comment
flash1293
left a comment
There was a problem hiding this comment.
LGTM with the comments above addressed. The CI failure is legit, kbn.table_list_view.listing.table.editActionName is not used anymore. Should be fixed if we implement the action like it is implemented in EUI - kbn.table_list_view.listing.table.editActionName is the aria-label of the button, kbn.table_list_view.listing.table.editActionDescription is the tooltip content.
|
We discussed this morning, and think that we can get away with an all-or-nothing action list (e.g. if you have edit permission on visualize, you get it on Lens), so I'm going to revert the more controversial bit of this PR. |
…ana into visualize/app-extensions
💔 Build Failed |
💔 Build Failed |
flash1293
left a comment
flash1293
left a comment
There was a problem hiding this comment.
Code LGTM, didn't test again
💔 Build Failed |
💚 Build Succeeded |
6 participants
This PR introduces a mechanism that allows other Kibana apps (such as Lens) to register visualizations with the Visualize app. These visualizations have their own saved objects and their own separate editing experiences, but still managed in the Visualize app's visualization list.