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] Improve tick formatting #97100

Closed
flash1293 opened this issue Apr 14, 2021 · 5 comments
Closed

[Lens] Improve tick formatting #97100

flash1293 opened this issue Apr 14, 2021 · 5 comments
Assignees
Labels
enhancement New value added to drive a business result Feature:Lens impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@flash1293
Copy link
Contributor

Right now the chart decides where to place tick labels on an axis, then the formatter of the current dimension is applied to the value. This has some edge cases leading to misleading charts

Screenshot 2021-04-14 at 11 34 10

Using the default format, it's showing correctly:
Screenshot 2021-04-14 at 11 34 53

I think the issue here is the following: The chart wants to place a tick at 3.5 - based on the formatter, this gets shown as 4, but at the position of 3.5 (which is highly confusing), It would place another tick at 4, which would be formatted correctly, but as elastic-charts is filtering out duplicate ticks, it's omitting this tick, resulting in the situation that got reported by the customer.

For a workaround, the user can make sure the formatting supports digits after the decimal point. It should work fine by default), maybe they changed the space wide advanced setting format:number:defaultPattern? They can also change it within Lens
Screenshot 2021-04-14 at 11 37 50

Also, see #92152

There are some ways we could tackle this ( via @markov00 ):

  • we internally have a property for each axis to only show integer numbers integerOnly. This truncate all the digits correctly
  • we can make the chart aware of the formatting type (you will just pass in a formatting string depending on the type) instead of a formatting function. In this way, the chart is aware of the "context" and knows how to deal with it. It is anyway a bit more complicated due to the fact that we need to align and define a set of interchangeable formatting patterns between Kibana and charts.
  • Lens could fix that, if instead of using rounding everything like 0.5.toFixed(0) it can truncate the number to the number of digits specified. Using floor for example when the number of digits is 0

It gets even worse because Lens doesn't own formatting either in most cases (it's globally configured in advanced settings and in the index pattern).

Because of this I think it would make sense to explore option 2 here and pass some information about the formatting to the chart so it can influence tick placement.

One possible API would be to extend the field formatter API with something like this:

getValueHint(): undefined | 'integer' | 'binary' | 'double'

Some (but not all) formatters are able to provide this information - Lens could pass it on to the chart which uses it to place ticks accordingly.

What do you think @markov00

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

Pinging @elastic/kibana-app (Team:KibanaApp)

@markov00
Copy link
Member

Before adding/changing the way field formatter works or include that part inside the chart itself I would prioritize this #8583 work, to avoid having a again another way to describe the formatting for numbers and currencies

@wylieconlon
Copy link
Contributor

@flash1293 this issue is overlapping with #57384, but I think this one is more about a specific plan.

@markov00 We've already done 3/4 of the changes I proposed in that issue. The main thing we haven't done is define a future-facing API that can solve problems like this one.

For tick formatting specifically, I like what you're proposing @flash1293 of adding some kind of metadata about what the values represent, like binary/decimal/time units. Here's the plan I would propose that we own:

  1. Since we already maintain a fork ofelastic/numeral-js, someone on Lens can contribute a new extractMetadata function that will convert a pattern string to metadata. For example 0.00bb could produce an output { units: 'bytes', base: 'binary', minDecimals: 2, maxDecimals: 2, commas: false }
    • This metadata is also useful because it lets us migrate to a JSON based format, like Numbro did with their fork of numeral
  2. Once we build the metadata extractor, someone from Lens can contribute a new function on the Field Formatter class which will be available on the field formatter instance. Something like format.getMetadata()
  3. Lens can use the metadata when building its chart display

The nice thing about this plan is that it also gives us a migration path away from pattern strings.

@nickofthyme
Copy link
Contributor

This issue is still present in lastest main branch.

Screen Recording 2023-10-18 at 05 21 46 PM

Notice in the recording above the value at 13:40 is 1.60, but when setting the decimals value to 0, the value becomes 2 and the axis tick label for 1.6 becomes 2, with no change in the axis scale itself. This is wrong and this elastic/elastic-charts#1417 was never truly fixed, so I reopened it. It worth noting this case only happens when formatting at a small scale such that decimal tick values are shown.

Really we need to format the value then somehow parse the formatted value to compare the value with the true value. Or maybe there is another way to take the lower/higher formatted value and remove the rest. Will continue this discussion in elastic/elastic-charts#1417.


Aside from the axis issue above - This issue will remain present in the value formatting across other parts of the chart such as the formatted tooltip/legend value and the actual rendered value.

Image 2023-10-18 at 5 33 02 PM

If you take the example above say the axes is fixed and with decimals set to 0 you should just see 1 and 2 axes ticks. If you were to hover again over the value at 13:40 it would read 2 even though the rendered placement is true to the actual scale and thus the point is clearly below the 2 axis tick.

I'm actually not sure how we fix this other than having an * to denote these types of rounded values.

So even if we fix this in the axis it is another topic of discussion of how we treat this in other parts of the chart. Not sure if we open a separate issue for that discussion.

@nickofthyme
Copy link
Contributor

Closing this issue in favor of #170151 and #170154

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 impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

6 participants