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

[Lens] Thresholds: ability to specify text as alternative to icons #112921

Closed
Tracked by #57706
dej611 opened this issue Sep 23, 2021 · 15 comments · Fixed by #113629
Closed
Tracked by #57706

[Lens] Thresholds: ability to specify text as alternative to icons #112921

dej611 opened this issue Sep 23, 2021 · 15 comments · Fixed by #113629
Assignees
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@dej611
Copy link
Contributor

dej611 commented Sep 23, 2021

Describe the feature:

From the uptime use case they want to recreate something like the following with Lens:

image

Note that it requires to annotate text instead of the icon marker on the axis && change the default placing of the marker (which ought to be on the bottom here).

cc @shahzad31

@dej611 dej611 added enhancement New value added to drive a business result Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Sep 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

@ghudgins
Copy link
Contributor

the icon / text "placement" might be better as a setting (above / below or right / left). I think the user might want to choose where the notation is as the icon on the axis helps in multiple axis cases (although still wondering if anyone will actually want to do that...)

@dej611
Copy link
Contributor Author

dej611 commented Sep 29, 2021

@MichaelMarcialis would you propose a specific design to fit the text marker?
Main concerns, to me, are:

  • should we allow exclusive icon vs text or both?
  • where the truncation should be applied? Upfront in the tet input or rather on the visualiation at the end?

@MichaelMarcialis
Copy link
Contributor

should we allow exclusive icon vs text or both?

As long as Elastic Charts supports both simultaneously (which I believe it does), I don't see any reason not to allow both. Thoughts, @ghudgins?

where the truncation should be applied? Upfront in the tet input or rather on the visualiation at the end?

I think this depends on what sort of truncation we intend to implement. If it will be some form of dynamic truncation (ex. one that takes into account the visualization canvas size and proximity to other thresholds), I imagine we could only apply that on the visualization side. If instead it will be something simple (ex. a character limit), I think we should convey that in the configuration input.

Additionally, is the assumption for this feature that we will only be allowing entry of a static text string? Or is there a desire to allow users to add variables in the string (such as the value, etc.)? And in either case, should this text input be separate from the "Display name" input? Or should we have the "Display name" input perform double duty as the text for both the visualization's threshold and in the dimension item in the layers panel?

@ghudgins
Copy link
Contributor

ghudgins commented Sep 29, 2021

should we allow exclusive icon vs text or both?

++ as long as there's room this makes sense.

and in either case, should this text input be separate from the "Display name" input? Or should we have the "Display name" input perform double duty as the text for both the visualization's threshold and in the dimension item in the layers panel?

In weekly last week I believe we had envisioned this as a re-use of the display name for the threshold. Value might be cool too for dynamic thresholds but I don't think it needs to be in this iteration if that helps you make a call.

@MichaelMarcialis
Copy link
Contributor

@ghudgins: Sounds good. So in summary:

  • Support for threshold icon, text, or both simultaneously.
  • Add an EuiSwitch to optionally render the current Display name on this threshold in the visualization. Default will be off.
  • Display name will continue to be a static text string (though we may consider supporting injection of dynamic variable values at a later time).
  • Since we are repurposing the Display name as optional threshold text, it wouldn't make sense to truncate up front on the configuration input (as Display name is used in multiple places, and not all will be truncated). That said, I'd still like to know how the truncation is to be performed.

Does that sound good to everyone? If so, let me know if you'd like a formal design for these changes, or if the items described above are enough to go on for development, @dej611.

@dej611
Copy link
Contributor Author

dej611 commented Sep 30, 2021

Ok, so I think I have almost everything I need to complete this task.

The last question I have is about label positioning: should the label positioning change with the presence/absence of the icon, or not?

Fluid marker layout:
Screenshot 2021-09-30 at 15 42 11

Fixed marker layout:
Screenshot 2021-09-30 at 15 46 56

cc @MichaelMarcialis @ghudgins @monfera @markov00

@ghudgins
Copy link
Contributor

ghudgins commented Oct 1, 2021

I think fixed covers the mixed icon / no icon case better. but i'd like if there are no icons at all to not have the wasted space in the "fixed" scenario (if that makes sense)

@dej611
Copy link
Contributor Author

dej611 commented Oct 1, 2021

I think fixed covers the mixed icon / no icon case better. but i'd like if there are no icons at all to not have the wasted space in the "fixed" scenario (if that makes sense)

Makes sense.

@dej611 dej611 self-assigned this Oct 1, 2021
@MichaelMarcialis
Copy link
Contributor

+1 to what @ghudgins said.

@markov00
Copy link
Member

markov00 commented Oct 4, 2021

What about moving the text inside the chart, above/below the line?

Screenshot 2021-09-22 at 19 41 18

For horizontal line it definitely improves the readability of the text
For the vertical line, you can let the user choose the text direction (horizontal/vertical)
And you can keep the icon outside if you need that

Also centering the text is not always the perfect choice: if you are defining a vertical threshold, the text could be left-aligned to the line to specify that "everything gt this line has is above the threshold"

@dej611
Copy link
Contributor Author

dej611 commented Oct 4, 2021

While I like the mockup with the text marker above/below the line, I have few questions around the following topics:

  • how to deal with bar value labels, to avoid collision or confusion between the two?
  • what automatic criteria works best to decide where to put the label in case of a line (Above/below/left/right)?
  • how to deal with area/rect annotations? Should the text be on the "other" side of the annotated area?

@markov00
Copy link
Member

markov00 commented Oct 4, 2021

  • how to deal with bar value labels, to avoid collision or confusion between the two?

offer the ability to move the text outside the chart?

  • what automatic criteria works best to decide where to put the label in case of a line (Above/below/left/right)?

Probably by default render text above(top) for horizontal lines, and on the right for the vertical ones. With the standard axis position this should not interfere much with the chart and also is in line with the current domain's directions. I will anyway let the user change a particular position if it looks better for them in a different position

  • how to deal with area/rect annotations? Should the text be on the "other" side of the annotated area?

For rect annotation, probably always align internally top/right? I'm not sure we have the ability to add text/markers for rect annotation right now

@ghudgins
Copy link
Contributor

ghudgins commented Oct 4, 2021

offer the ability to move the text outside the chart?

@markov00 I like the suggestion. If we foresee we'll need to have an option where the reference line label is outside of the visualization could we keep the implementation we have now and make an issue for rect reference line annotations? Is that the right way forward on this?

Should the text be on the "other" side of the annotated area?

Looks like it. If you have one, it shows on the "other" side. If you have both, it shows on the same side. Same logic we did for the icons / text today....use the space if you have it.

As an aside, one detail that I quite like from @markov00's screenshot is the matching of the reference line color to the axis color. It's the first example I've seen that actually is easy to parse even though there's a lot happening. We should check to see if we can specify axis color at some point (not sure we have an issue for this...will look).

@dej611
Copy link
Contributor Author

dej611 commented Oct 4, 2021

I'm not sure we have the ability to add text/markers for rect annotation right now

Yes, it is possible to have it. Or at least it works already with the Lens implementation 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants