Skip to content
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

[Wordcloud] Replace kibana tagcloud with es-charts #1156

Closed
6 tasks done
stratoula opened this issue May 11, 2021 · 10 comments
Closed
6 tasks done

[Wordcloud] Replace kibana tagcloud with es-charts #1156

stratoula opened this issue May 11, 2021 · 10 comments
Labels
enhancement New feature or request kibana cross issue Has a Kibana issue counterpart released Issue released publicly :wordcloud Wordcloud related issues

Comments

@stratoula
Copy link

stratoula commented May 11, 2021

Is your feature request related to a problem? Please describe.
I am currently working on replacing the kibana tagcloud viz with es-charts and I want to report some gaps that exist between the two implementations that need to be addressed.

  • onElementClick: I want to be able to click on a word and get the clicked value. Right now I get:
{
    "smAccessorValue": "",
    "groupByRollup": "Word count",
    "value": 9,
    "sortIndex": 0,
    "path": [],
    "depth": 0
}

every time I click a word. I would like to get the clicked term. This is needed because we want to trigger the filter action every time a term is clicked.. Moreover, I see that the click event is triggered everywhere and not only when I click a term. (I am fine with it as soon as no term is returned when I click on a blank space).

  • I need to be able to display/hide the label. This label comes from the configuration. It contains the bucket field, the ordering and the metric. I could construct it on the kibana side and just pass it as a prop to the chart. We can further discuss it.

tagcloud

  • Functional tests. I need to check for the following conditions:
  • get all the rendered terms
  • get the font sizes per term
  • be able to select a tag and click it (I can overpass it by clicking a hardcoded x and y but if we can have this info would be awesome)

Kibana Cross Issues
elastic/kibana#95539

Checklist

Delete any items that are not applicable to this feature request.

  • this request is checked against already exist requests
  • every related Kibana issue is listed under Kibana Cross Issues list
  • kibana cross issue tag is associated to the issue if any kibana cross issue is present
@stratoula stratoula added enhancement New feature or request kibana cross issue Has a Kibana issue counterpart :wordcloud Wordcloud related issues labels May 11, 2021
@monfera
Copy link
Contributor

monfera commented May 11, 2021

Thank you Stratoula!

  1. Label
    @markov00 might have a different take, at the moment my preference would be that Kibana supply the chart label (I'd call it a title, but maybe there's a distinct chart title in Kibana above the chart?) because it probably needs application specific labeling (if at all), and its contents are also application specific (eg. should show some total count or not); also, this way there's clarity about internationalization. In fact it may even be preferable to overlay the chart with your own label, because then it'll be inherently part of the accessibility approach on the recipient page. There's also the question of handling text overrun (won't ever happen, let's not bother with it? or truncate with ellipsis? etc.). Looking for your and Marco's feedback on this.

  2. Padding
    The placement of the label requires space, and assuming that the label render responsibility is with elastic-charts, it needs to set aside the appropriate area. It can be handled internally. Even then, is there expectation about leaving a gap of a specific pixel count or chart height ratio between wordcloud proper and title, and below the title?

  3. Click event handling
    Does the functionality of clicking on a specific word, and getting a callback for just that word work in Kibana? I'm asking it because there are some intricacies. Examples:
    very small words might be placed in the crevices between letters of large words
    moreover, very small words can even be placed inside other letters, eg. in the letter O, u, etc. if there's enough space
    the click doesn't have unambiguous semantics: is it the text ink only? or, for better adherence to Fitts law, the bounding box? but in the latter case, bounding boxes may easily overlap even setting aside the above concerns, because the placement observes the contour of the text, not the bounding box
    is the current code doing something to handle overlaps (on the offchance it's not ink based or voronoi based), eg. returning an array of two or more elements?

@stratoula
Copy link
Author

stratoula commented May 11, 2021

Thanx Robert for the reply!

About the title/label: I can share with you how it currently works. It is always on the bottom of the viz and is responsive. So here you can see a panel with the panel title at the top (this is irrelevant with the viz, it is configured by the dashboard) and the label/title on the bottom. (The warning icon informs the user that the container is too small and some tags are hidden)
image
This case seems a little bit unrealistic from my point of view but we could keep this approach. Ellipsis also seems fine to me as it depends on the users if they want to display this or not. Right now there is a padding-top of 16px from the viz wrapper (no padding at the bottom) which seems to work fine. Here is a video that demonstrates how the responsive works.

tagcloud-resizer

Another idea is to not be part of the chart and to be added from us on the kibana side. Wdyt? (In case a title prop doesn't make any sense)

About the click event, right now it works with adding a click event on each word. This works pretty well, I haven't found any case of overlapping as the words never overlap and have a space of some pixels between them.
But even if it returns an array of words, we could create as many filters as the words.

@stratoula
Copy link
Author

Update: I implemented the chart label on the kibana side so no need to be supported on the chart.

@monfera
Copy link
Contributor

monfera commented May 12, 2021

Thanks, Stratoula! So the two pieces of work are, the click event handling, and the provisioning of data for test purposes in the data- attribute in the DOM. I'll discuss it with @markov00 next week. Btw. what target date are you looking for these two each, to be aligned with your and your team's priorities?

@monfera
Copy link
Contributor

monfera commented May 12, 2021

Btw. it's good to hear that the way you configure it in Kibana, there won't be word-in-word occurrence. (For an example of that, see the middle image in the PR from @Kati-dev-hu - there's text in the "o" of "xobni"). In this case, attaching standard mouse click event handlers on the DOM elements works well. It probably works for the text-in-text case too, as most likely the rendering is ordered in that the big word is layouted first, ie. rendered first, ie. the small item atop of it can still capture the mouse. The bottom right has other examples too. Though in general it'd be worthwile to use the added configurability at some point, to enable the denser look and angle variety seen in the PR.

@stratoula
Copy link
Author

stratoula commented May 13, 2021

Btw. what target date are you looking for these two each, to be aligned with your and your team's priorities?

It would be awesome if wecould replace the old tagcloud with the es-charts implementation on 7.14 but it is perfectly fine if we merge it on 7.15.

@markov00
Copy link
Member

my 2 cents:

  1. chart title: my preference is to keep it at the moment outside the chart library. You have much more flexibility then having a prop + all the possible styling options in charts
  2. click handler: if there is no word-in-word occurrences, then sorting first the rendering + attaching the DOM event to each element should work

@monfera
Copy link
Contributor

monfera commented May 13, 2021

Indeed, my understanding is that it's already ordered this way by the d3-cloud library, so I know of these two tasks:

-attaching the click event handlers onto the SVG <text> elements (now glad we went the DOM way on this chart)

  • provisioning of data for test purposes in the data- attribute in the DOM

@rshen91 as the vis accessibility powerhouse, it'd be worth taking an initial look at it, as the ARIA-* labels can be directly used on each DOM element, if needed

@stratoula
Copy link
Author

Another update about the functional tests. There is no need to provide me with all this info that I mentioned above. As wordcloud is based on svg I can access the elements and write the functional tests!
With that being said, the only thing that is missing is the click event per text node.

nickofthyme pushed a commit that referenced this issue Jun 4, 2021
# [30.0.0](v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([#1181](#1181)) ([76e8dca](76e8dca)), closes [#1129](#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](#1182)) ([a64f333](a64f333))
* annotation tooltip display when remounting specs ([#1167](#1167)) ([8408600](8408600))
* render nodeLabel formatted text into the nodes ([#1173](#1173)) ([b44bdff](b44bdff))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](#1145)) ([7c1fa8e](7c1fa8e))
* apply value formatter to the default legend item label ([#1190](#1190)) ([71474a5](71474a5))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](#1163)) ([380363b](380363b)), closes [#1108](#1108)
* **wordcloud:** click and over events on text ([#1180](#1180)) ([196fb6a](196fb6a)), closes [#1156](#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
@nickofthyme
Copy link
Collaborator

🎉 This issue has been resolved in version 30.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Jun 4, 2021
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
# [30.0.0](elastic/elastic-charts@v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([opensearch-project#1181](elastic/elastic-charts#1181)) ([92ba84c](elastic/elastic-charts@92ba84c)), closes [opensearch-project#1129](elastic/elastic-charts#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](elastic/elastic-charts#1182)) ([880fbf1](elastic/elastic-charts@880fbf1))
* annotation tooltip display when remounting specs ([opensearch-project#1167](elastic/elastic-charts#1167)) ([7163951](elastic/elastic-charts@7163951))
* render nodeLabel formatted text into the nodes ([opensearch-project#1173](elastic/elastic-charts#1173)) ([0de9688](elastic/elastic-charts@0de9688))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](elastic/elastic-charts#1145)) ([6787728](elastic/elastic-charts@6787728))
* apply value formatter to the default legend item label ([opensearch-project#1190](elastic/elastic-charts#1190)) ([20108bb](elastic/elastic-charts@20108bb))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](elastic/elastic-charts#1163)) ([b858fb3](elastic/elastic-charts@b858fb3)), closes [opensearch-project#1108](elastic/elastic-charts#1108)
* **wordcloud:** click and over events on text ([opensearch-project#1180](elastic/elastic-charts#1180)) ([adbf341](elastic/elastic-charts@adbf341)), closes [opensearch-project#1156](elastic/elastic-charts#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kibana cross issue Has a Kibana issue counterpart released Issue released publicly :wordcloud Wordcloud related issues
Projects
None yet
Development

No branches or pull requests

4 participants