[Canvas] Feat: static tags for elements #28779

cqliu1 · 2019-01-15T19:45:08Z

Phase 1 of #25532.
Closes #23172.

This adds static tags to Canvas element plugins (not custom elements yet...) and makes the element selection filterable by tags.

adds stories for Tag and TagList components
updates stories for ElementCard and ElementGrid
adds type def for @kbn/interpreter/Registry

Note: I added type definition files to Registry in the @kbn/interpreter package and typescriptified all the tag related files so CI would stop failing. Since I made changes to a package, you'll want to run yarn kbn bootstrap while checked out in this branch.

Tags added:

`chart` for the plot/pie/progress elements

`filter`

`graphic` for image and shape elements

`proportion` for elements that compare a part to a whole (pie, progress, reveal image, and repeat image)

`text`

elasticmachine · 2019-01-15T19:45:10Z

Pinging @elastic/kibana-canvas

elasticmachine · 2019-01-15T19:57:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-15T20:11:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

w33ble · 2019-01-15T20:15:30Z

One thing I've learned about tags is that less is more. I think we could cull the list a little (tags like "line", "point" and "bar" can probably go, likely others too).

cqliu1 · 2019-01-15T20:19:25Z

Yeah, I went a little overboard with the tags. Would it be helpful to differentiate between plot and pie or is chart sufficient?

w33ble · 2019-01-15T20:22:44Z

I think chart is enough, the others are already names of visualizations. I suspect users trying to filter with tags are more concerned with separating data and not data elements, and the chart and graphic tags do this already.

Shoot for the least amount of tags here, we can always add more later, but it's hard to remove them. And the more tags we have, the less useful they become.

ryankeairns · 2019-01-15T20:25:44Z

+1 on the small set of tags. Chart, text, graphic, and progress probably cover a lot and leave us with 1 per asset in most, if not all, cases.

This is looking great, I'll give it a spin locally.

cqliu1 · 2019-01-15T20:52:03Z

@w33ble @ryankeairns I trimmed down the list of tags and updated the screenshots in the description ^

w33ble · 2019-01-15T21:04:56Z

Looking a lot better. One question; isn't a progress element a type of chart element? And if you're intent on keeping the progress tag, wouldn't it make sense to add it to revealImage as well?

elasticmachine · 2019-01-15T21:07:08Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ryankeairns

Looking good, one small change regarding the card layout for badges.

ryankeairns · 2019-01-15T22:08:19Z

x-pack/plugins/canvas/public/components/element_types/element_types.js

+              <br />
+              <TagList tags={tags} tagType={tagType} />
+            </Fragment>
+          }


We can make the badges appear in the same place (bottom) of each card to tidy things up layout-wise. Just use the footer prop on EuiCard like so:

... description={ <Fragment> {help} </Fragment> } footer={ <TagList tags={tags} tagType={tagType} /> } ...

Good call! The spacing looks much nicer.

ryankeairns · 2019-01-15T22:15:09Z

One additional minor UI nitpick. The close icon overlaps the Tag selector just a bit (see below). A quick fix would be to give the .euiModalHeader one more pixel of padding-right or move the close icon over. With either solution, we would want to add a custom css class to apply that override as opposed to overriding any existing .eui... class names.

cqliu1 · 2019-01-15T22:47:26Z

@ryankeairns 1 pixel makes a big difference

ryankeairns

Thanks for making those changes, it's so close :) In the component and in the scss file modal is misspelled as model (e.g. canvasElements__modelHeader). Other than that, this looks good!

elasticmachine · 2019-01-15T23:02:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-15T23:14:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

w33ble · 2019-01-16T00:03:17Z

Arbitrary tag handling, nice!

w33ble

This is looking good. I like that you can use tag:whatever in the search bar, nice touch!

w33ble · 2019-01-15T23:58:08Z

x-pack/plugins/canvas/public/components/tag/tag.js

+  type: PropTypes.string.isRequired,
+};
+
+Tag.defaultProps = {


nit: you don't need to add default props when the propType is marked as required

w33ble

Bah, of course I find a bug after approving :(

If I search for a garbage string, I get no results, as expected:

But if I search for a garbage tag, it just shows everything:

Seems like a bug in the tag search/filter logic.

cqliu1 · 2019-01-16T21:55:16Z

@w33ble It should actually be tags: instead of tag: if you want to type in your tags. Typing in tags:foo returns 0 results.

Should I rename that field from tags to tag?

w33ble · 2019-01-16T22:13:55Z

Should I rename that field from tags to tag?

Nope, it's still a bug, but that's not it. On master, I can't even put a : in the search box, so this is definitely a new bug. It looks like if you put <anything>:<anything> you always get all of the results. Below I enter wat:, and get no results, but as soon as I add anything to the right of the colon, it just shows everything.

~~I don't know why it works this way. Maybe it's an EUI bug?~~ After some discussion, it does seem like a bug in this PR, somewhat related to the Query object that comes out of that component. We're planning to ignore that completely and just use the queryText and do our own parsing.

elasticmachine · 2019-05-07T20:47:57Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-07T21:19:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-07T22:34:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

monfera

Great work Catherine, fantastic PR 🎉

Super minor things, maybe to circle back to in some future PR:

the report tag is now yielding the empty set
'graphicandreport` have identical-looking colors
depending on the element count, a vertical scrollbar comes and goes, changing the width of the container

Currently, selecting multiple tags forms an intersection rather than a union of the elements - you have my vote here, despite that 1) there are few tags, and 2) they don't hugely overlap, so a union might work slightly better right now. On the other hand, when we eventually have more numerous, orthogonal tabs, a selection like timeline&area&ohlc&annotated would let a user select only those elements (from possibly many! idea of Canvas is eg. render plugins and clients may roll a big roster) that are timeline visualizers that can render areas OHLC bars and support user annotations. So, imo good future-proof choice there!

shaunmcgough

Hi @cqliu1 - This looks good. I basically noticed the same things that @monfera noticed. Quite minor things indeed. Great work!

cqliu1 · 2019-05-08T16:13:49Z

the report tag is now yielding the empty set

@monfera That's bc report exists as a tag on one of the workpad templates but not any of the elements. The tag filter is populated by the tag registry with both tags from templates and elements. I'm not sure if there's a nice way to separate the two lists, but this is similar to the behavior of labels on github where issues and PRs use labels from the same set.

ryankeairns

LGTM. A couple of minor things:

perhaps we should remove 'presentation' from the dropdown since there aren't any elements using that tag
something to keep an ear open for - do people expecting selecting multiple tags to be an "OR" situation as opposed to an "AND". Currently, it if you select multiple tags it only shows elements that have all those tags. I expected it to show all elements with either tag, but this feels like a 50/50 so we can just ship and see what feedback we get.

cqliu1 · 2019-05-08T16:30:09Z

@ryankeairns @monfera @shaunmcgough Thanks for the feedback! I chose to "AND" the tags filters, but that's the behavior I expected and what I saw on github, but it may not be expected for other users. If we get complaints about the behavior, we can always switch to "OR" or maybe someone let the user choose "OR" or "AND".

RE: the list of tags in the filter, the tag list is populated with the entire tag registry which registers element and workpad template tags. We can consider filtering the list of tags with the tags that exist in the object they're filtering.

elasticmachine · 2019-05-08T18:16:22Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…lates Added tag filtering to element selection modal Added tags to elements Removed unused tags Fixed spacing in element card Pruned out extraneous tags Added padding to element types modal header. Moved tags to EuiCard footer Fixed typo Fixed prop types Replaced progress with proportion tag. Updated tags Fixed search bar behavior in element picker and workpad templates Added type definitions to Registry in @kbn/interpreter Typescriptified tags Renamed 'tags' field to 'tag' in EuiSearchBar filter config Fixed import paths Added tags filter to ElementTypes Converted tag component Added stories for Tag and TagList components

elasticmachine · 2019-05-08T20:20:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Extract Tags component and tag filter config object from workpad templates Added tag filtering to element selection modal Added tags to element Added type definitions to Registry in @kbn/interpreter Added stories for Tag and TagList components Changed graphic tag color

cqliu1 added review v7.0.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.7.0 labels Jan 15, 2019

cqliu1 requested review from w33ble and ryankeairns January 15, 2019 19:45

cqliu1 requested a review from a team as a code owner January 15, 2019 19:45

ryankeairns requested changes Jan 15, 2019

View reviewed changes

cqliu1 requested a review from a team as a code owner January 15, 2019 22:48

ryankeairns requested changes Jan 15, 2019

View reviewed changes

w33ble approved these changes Jan 16, 2019

View reviewed changes

w33ble suggested changes Jan 16, 2019

View reviewed changes

cqliu1 force-pushed the feat/element-tags branch from 9f74380 to 391f3f7 Compare January 16, 2019 21:59

cqliu1 requested review from ryankeairns, shaunmcgough and monfera May 7, 2019 20:35

monfera approved these changes May 8, 2019

View reviewed changes

shaunmcgough approved these changes May 8, 2019

View reviewed changes

ryankeairns approved these changes May 8, 2019

View reviewed changes

cqliu1 added the release_note:enhancement label May 8, 2019

cqliu1 added 5 commits May 8, 2019 12:08

More ts

d3a1396

Fixed ts errors

c230fe0

More cleanup

f598683

Changed graphic tag color

c1900ef

cqliu1 force-pushed the feat/element-tags branch from fc62d5b to c1900ef Compare May 8, 2019 19:09

cqliu1 merged commit 9c6a58c into elastic:master May 8, 2019

cqliu1 mentioned this pull request May 8, 2019

[7.x] [Canvas] Feat: static tags for elements (#28779) #36307

Merged

bhavyarm mentioned this pull request May 28, 2019

Text search should work for tags on element search box in canvas #37282

Closed

rayafratkina mentioned this pull request Sep 13, 2019

saved object tagging phase 1 #16484

Closed

fktkrt mentioned this pull request Dec 14, 2019

Need a better way to organize visualizations and dashboards #53046

Closed

cqliu1 deleted the feat/element-tags branch May 6, 2020 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Canvas] Feat: static tags for elements #28779

[Canvas] Feat: static tags for elements #28779

cqliu1 commented Jan 15, 2019 •

edited

Loading

elasticmachine commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

w33ble commented Jan 15, 2019

cqliu1 commented Jan 15, 2019 •

edited

Loading

w33ble commented Jan 15, 2019

ryankeairns commented Jan 15, 2019

cqliu1 commented Jan 15, 2019

w33ble commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

ryankeairns left a comment

ryankeairns Jan 15, 2019

cqliu1 Jan 15, 2019

ryankeairns commented Jan 15, 2019

cqliu1 commented Jan 15, 2019

ryankeairns left a comment

elasticmachine commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

w33ble commented Jan 16, 2019

w33ble left a comment

w33ble Jan 15, 2019

w33ble left a comment

cqliu1 commented Jan 16, 2019

w33ble commented Jan 16, 2019 •

edited

Loading

elasticmachine commented May 7, 2019

elasticmachine commented May 7, 2019

elasticmachine commented May 7, 2019

monfera left a comment

shaunmcgough left a comment

cqliu1 commented May 8, 2019

ryankeairns left a comment

cqliu1 commented May 8, 2019 •

edited

Loading

elasticmachine commented May 8, 2019

elasticmachine commented May 8, 2019

[Canvas] Feat: static tags for elements #28779

[Canvas] Feat: static tags for elements #28779

Conversation

cqliu1 commented Jan 15, 2019 • edited Loading

Tags added:

chart for the plot/pie/progress elements

filter

graphic for image and shape elements

proportion for elements that compare a part to a whole (pie, progress, reveal image, and repeat image)

text

elasticmachine commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

💔 Build Failed

elasticmachine commented Jan 15, 2019

💔 Build Failed

w33ble commented Jan 15, 2019

cqliu1 commented Jan 15, 2019 • edited Loading

w33ble commented Jan 15, 2019

ryankeairns commented Jan 15, 2019

cqliu1 commented Jan 15, 2019

w33ble commented Jan 15, 2019

elasticmachine commented Jan 15, 2019

💔 Build Failed

ryankeairns left a comment

Choose a reason for hiding this comment

ryankeairns Jan 15, 2019

Choose a reason for hiding this comment

cqliu1 Jan 15, 2019

Choose a reason for hiding this comment

ryankeairns commented Jan 15, 2019

cqliu1 commented Jan 15, 2019

ryankeairns left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 15, 2019

💔 Build Failed

elasticmachine commented Jan 15, 2019

💔 Build Failed

w33ble commented Jan 16, 2019

w33ble left a comment

Choose a reason for hiding this comment

w33ble Jan 15, 2019

Choose a reason for hiding this comment

w33ble left a comment

Choose a reason for hiding this comment

cqliu1 commented Jan 16, 2019

w33ble commented Jan 16, 2019 • edited Loading

elasticmachine commented May 7, 2019

💚 Build Succeeded

elasticmachine commented May 7, 2019

💔 Build Failed

elasticmachine commented May 7, 2019

💚 Build Succeeded

monfera left a comment

Choose a reason for hiding this comment

shaunmcgough left a comment

Choose a reason for hiding this comment

cqliu1 commented May 8, 2019

ryankeairns left a comment

Choose a reason for hiding this comment

cqliu1 commented May 8, 2019 • edited Loading

elasticmachine commented May 8, 2019

💔 Build Failed

elasticmachine commented May 8, 2019

💚 Build Succeeded

cqliu1 commented Jan 15, 2019 •

edited

Loading

`chart` for the plot/pie/progress elements

`filter`

`graphic` for image and shape elements

`proportion` for elements that compare a part to a whole (pie, progress, reveal image, and repeat image)

`text`

cqliu1 commented Jan 15, 2019 •

edited

Loading

w33ble commented Jan 16, 2019 •

edited

Loading

cqliu1 commented May 8, 2019 •

edited

Loading