Click to filter values in Discover doc table and Visualize data table #9989

Bargs · 2017-01-20T23:01:34Z

Fixes #3432
Replaces #5344

This PR extends the filter icons implemented in #5344 to the data table visualization.

lukasolson · 2017-01-20T23:08:00Z

src/ui/public/directives/rows.js

+                ></span>
+              </span>
+            </td>`
+          );


Could you move this into its own template file?

lukasolson · 2017-01-20T23:09:49Z

src/ui/public/directives/rows.js

            if ($(event.target).is('a')) return; // Don't add filter if a link was clicked
-            clickHandler({ point: { aggConfigResult: aggConfigResult } });
+            clickHandler({ point: { aggConfigResult: aggConfigResult }, negate: negate });


The style guide suggests using the property value shorthand syntax (simply negate instead of negate: negate).

jimmyjones2 · 2017-01-22T08:36:30Z

This is awsome @Bargs, thanks!

Two minor inconsistencies between discover and the data table - whole row in data table is highlighted on mouseover and magnifying glass shown at the top of the row.

Bargs · 2017-01-22T18:34:49Z

Thanks for the input @jimmyjones2! The cell/row highlighting was an existing style of the data table. Perhaps you're right we should remove it now. What do you think @cjcenizal?

Could you provide a screenshot of the positioning inconsistency you mentioned? From what I could tell the doc table also put the magnifying icon at the top of the row but I may have missed something.

jimmyjones2 · 2017-01-23T17:48:36Z

@Bargs Sorry I wasn't very clear - in the header of the data table there is a magnifying glass as a cue for filtering, but not in the doc table.

cjcenizal · 2017-01-23T21:01:47Z

This looks good from a UX perspective, except for one weird thing I found. If I tried to filter on a value, sometimes it tried to apply a filter for every value in the row, instead of just the value of the cell I clicked on. When this happened, it prompted me to verify that I want to filter. If I selected a different cell, this behavior didn't occur, and instead the filter was applied for just the cell I clicked on. From a UX perspective, I couldn't figure out why this was happening. Any idea what's going on here?

cjcenizal · 2017-01-23T21:05:35Z

src/ui/public/doc_table/components/table_row.js

-            formatted: _displayField(row, column, true)
+            formatted: _displayField(row, column, true),
+            filterable: isFilterable,
+            column: column


This can also just be column, similar to @lukasolson's comment.

Bargs · 2017-01-23T21:09:38Z

@cjcenizal this is the way the current filtering works in the data table ¯_(ツ)_/¯

cjcenizal · 2017-01-23T22:24:06Z

@Bargs Gotcha thanks. I dug into this a bit and created a ticket for improving this UX: #10024

I agree that we should remove the hover highlight, since we no longer need it to signify that the cell is clickable.

ppisljar · 2017-01-24T07:59:21Z

use hover to highlight whole row then ?

cjcenizal · 2017-01-24T16:37:31Z

@ppisljar I'm open to trying that out. What do you think the biggest benefit(s) of that is for the user?

ppisljar · 2017-01-24T19:11:11Z

with multiple columns its hard to see which value matches to the row you are looking to, highlighting the row makes it obvious.

Bargs · 2017-01-25T21:49:41Z

@lukasolson @cjcenizal @ppisljar Just pushed some updates:

Data table now only highlights row on hover, not individual cell
Fixed filter icon colors in dark theme
Extracted filter html to a template
Use object shorthand in a couple of places
Removed outdated filter tool tip at the top of data table columns

Please take another look.

cjcenizal

LGTM!

Bargs · 2017-01-27T16:14:14Z

jenkins, test this

lukasolson · 2017-01-28T00:28:47Z

Hmm, how complicated would it be to centralize this logic into a directive and re-use it in both places? That way we wouldn't have the HTML duplicated in two different places... What do you think?

Bargs · 2017-01-30T19:44:46Z

@lukasolson I think a bit of it could be DRYed up, but not a ton. The way the filters are built is quite different in each context, so it would really just be the html that gets de-duplicated. I don't mind doing it, but I also know @ppisljar is working on some major changes to the data table so it might be best to avoid sharing this logic for now.

lukasolson

LGTM

… & + & selector

…elastic#9989) Adds + and - icons on hover in the doc table and data table, allowing users to drill down with more options and fewer clicks. Previously in the doc table a user had to expand the row in order to filter on a document value. In the data table a user could filter simply by clicking a cell, but there was no way to create a negative filter. This commit brings feature parity between the data table and doc table.

…#9989) (#10123) Adds + and - icons on hover in the doc table and data table, allowing users to drill down with more options and fewer clicks. Previously in the doc table a user had to expand the row in order to filter on a document value. In the data table a user could filter simply by clicking a cell, but there was no way to create a negative filter. This commit brings feature parity between the data table and doc table.

marenas · 2017-02-03T00:27:15Z

src/ui/public/doc_table/components/table_row.js

@@ -128,7 +147,7 @@ module.directive('kbnTableRow', function ($compile) {
          // rebuild cells since we modified the children
          $cells = $el.children();

-          if (i === 0 && !reuse) {
+          if (!reuse) {
            $toggleScope = $scope.$new();
            $compile($target)($toggleScope);


@Bargs : Sorry for the late and random comment, but doesn't this $compile($target)($toggleScope); on all "newHtmls" elements for each row introduce some performance degradation when showing many rows ?

@marenas I haven't personally seen any slowdown with this change. The template for each cell is pretty minimal.

makes sense, thanks !

Bargs added review v5.3.0 v6.0.0 labels Jan 20, 2017

Bargs assigned lukasolson and cjcenizal Jan 20, 2017

Bargs requested review from lukasolson and cjcenizal January 20, 2017 23:01

Bargs mentioned this pull request Jan 20, 2017

Click to filter values in Discover doc table #5344

Closed

lukasolson requested changes Jan 20, 2017

View reviewed changes

cjcenizal reviewed Jan 23, 2017

View reviewed changes

tbragin added the :Discovery label Jan 26, 2017

cjcenizal approved these changes Jan 26, 2017

View reviewed changes

lukasolson approved these changes Jan 30, 2017

View reviewed changes

jimmyjones2 added 4 commits January 30, 2017 16:59

Click to filter values in Discover doc table

ea0d544

Changes suggested by @cjcenizal

da47b23

Un-nest selector to avoid unnecessary specifity and fix bug caused by…

ef6b05c

… & + & selector

Move flattenedRow to the scope as suggested by @Bargs

55ba60f

Bargs added 9 commits January 30, 2017 16:59

Fix lint error

92476c4

Add filter icons to data table cells

2d73dff

Only highlight row on hover

974fd22

Fix dark theme filter icon background color

56bb699

More dark theme fixes

465aa62

extract filter html into a template

2d13dfe

Use object key shorthand per styleguide

f03ffcf

Remove outdated advice

f06b666

object shorthand

86c1862

Bargs force-pushed the rowFilters branch from fd8afb6 to c30ce5a Compare January 30, 2017 22:14

Fix selenium tests by avoiding newline created by block element

88a9c0d

Bargs force-pushed the rowFilters branch from c30ce5a to 88a9c0d Compare January 31, 2017 16:25

Bargs merged commit 4da7d8a into elastic:master Jan 31, 2017

marenas reviewed Feb 3, 2017

View reviewed changes

Bargs mentioned this pull request Feb 10, 2017

Provide a way to expand events in Discover by clicking anywhere on the row #4976

Closed

bhavyarm mentioned this pull request Mar 15, 2017

Clicking on - filter on data table does not filter out the value. #10769

Closed

cjcenizal mentioned this pull request Apr 26, 2017

[DataTable] No lines shown between terms from the bucket. #11423

Closed

Bargs mentioned this pull request Apr 27, 2017

Filtering out on spy panel in discover doesn't work. #11470

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Click to filter values in Discover doc table and Visualize data table #9989

Click to filter values in Discover doc table and Visualize data table #9989

Bargs commented Jan 20, 2017

lukasolson Jan 20, 2017

lukasolson Jan 20, 2017

jimmyjones2 commented Jan 22, 2017

Bargs commented Jan 22, 2017

jimmyjones2 commented Jan 23, 2017

cjcenizal commented Jan 23, 2017

cjcenizal Jan 23, 2017

Bargs commented Jan 23, 2017 •

edited

cjcenizal commented Jan 23, 2017

ppisljar commented Jan 24, 2017

cjcenizal commented Jan 24, 2017

ppisljar commented Jan 24, 2017

Bargs commented Jan 25, 2017

cjcenizal left a comment

Bargs commented Jan 27, 2017

lukasolson commented Jan 28, 2017

Bargs commented Jan 30, 2017

lukasolson left a comment

marenas Feb 3, 2017 •

edited

Bargs Feb 3, 2017

marenas Feb 3, 2017

Click to filter values in Discover doc table and Visualize data table #9989

Click to filter values in Discover doc table and Visualize data table #9989

Conversation

Bargs commented Jan 20, 2017

lukasolson Jan 20, 2017

Choose a reason for hiding this comment

lukasolson Jan 20, 2017

Choose a reason for hiding this comment

jimmyjones2 commented Jan 22, 2017

Bargs commented Jan 22, 2017

jimmyjones2 commented Jan 23, 2017

cjcenizal commented Jan 23, 2017

cjcenizal Jan 23, 2017

Choose a reason for hiding this comment

Bargs commented Jan 23, 2017 • edited

cjcenizal commented Jan 23, 2017

ppisljar commented Jan 24, 2017

cjcenizal commented Jan 24, 2017

ppisljar commented Jan 24, 2017

Bargs commented Jan 25, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Bargs commented Jan 27, 2017

lukasolson commented Jan 28, 2017

Bargs commented Jan 30, 2017

lukasolson left a comment

Choose a reason for hiding this comment

marenas Feb 3, 2017 • edited

Choose a reason for hiding this comment

Bargs Feb 3, 2017

Choose a reason for hiding this comment

marenas Feb 3, 2017

Choose a reason for hiding this comment

Bargs commented Jan 23, 2017 •

edited

marenas Feb 3, 2017 •

edited