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

Add Numeral.js format pattern to Color format of number #8565

Closed
mrvincenzo opened this issue Oct 6, 2016 · 17 comments
Closed

Add Numeral.js format pattern to Color format of number #8565

mrvincenzo opened this issue Oct 6, 2016 · 17 comments
Labels
Feature:FieldFormatters Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:enhancement

Comments

@mrvincenzo
Copy link

Add Numeral.js format pattern to Color format like in Number format. It would be nice to be able to both choose colors/ranges and specify the way the numeric value appears (i.e. 0,0.[000]).

@thomasneirynck
Copy link
Contributor

I agree, this is a good way forward to address #8564 as well.

@thomasneirynck thomasneirynck changed the title Feature: Add Numeral.js format pattern to Color format of number Add Numeral.js format pattern to Color format of number Oct 6, 2016
@thomasneirynck thomasneirynck self-assigned this Oct 6, 2016
@epixa
Copy link
Contributor

epixa commented Oct 7, 2016

It's important that we don't just defer formatting capabilities to a third party library like numeral.js, otherwise the behavior and capabilities of a feature of Kibana is defined by the API of that dependency, which isn't acceptable.

That isn't meant to discourage the use of numeral.js for this, but it must be an implementation detail of a concrete and clear abstraction that we have complete control over. The alternative is to provide configurable formatters that people could drop in in the event that we need to move away from numeral.js but they need to continue to rely on it, but I don't think that's appropriate in this case.

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Oct 7, 2016

@epixa we should probably revisit all the other formatting then as well (percent, bytes, numbers, ...) they all rely on the same formatting pattern from numeral.js

@epixa
Copy link
Contributor

epixa commented Oct 7, 2016

@thomasneirynck Yeah, I'd say that's pretty high priority. At the moment, we risk breaking backwards compatibility for users whenever we bump that library at all, which is especially scary in the event that there were a critical security issue or something that we'd need to bump numeral.js for, which would be done in a patch version release of Kibana.

@insukcho
Copy link

insukcho commented Apr 9, 2018

Any update about this?

We have a double field which uses color formatting. However, this overwrites the default number format of 3 digits after the decimal point, so far. My version is 6.2.

@fbaligand
Copy link
Contributor

@epixa
Well, numeral.js format is everywhere in Kibana since Kibana 4.0.
It is also used in recent new features like TSVB or numeric locale in advanced settings.
Replace numeraljs would be really breaking for every kibana user.
Furthermore, I don’t think there is a particular risk since numeraljs lib used in Kibana is @elastic/numeraljs, so it is already a fork managed by Elastic team (especially @spalger ).
So it is nearly as it is included in kibana code.

So that would be great to add a « format » input (based on numeraljs) when type is number.
That would be coherent with existing kibana features.

@epixa
Copy link
Contributor

epixa commented Jul 12, 2018

@fbaligand I hear what you're saying, but our usage (including and especially the fork) of numeral.js as a big problem that we need to fix. We're already talking about how best to go about ripping all of that out, and the single biggest reason it's so hard is because we're using it for so many features. Just piling onto that usage doesn't seem prudent.

@fbaligand
Copy link
Contributor

fbaligand commented Jul 12, 2018

Thanks @epixa for your answer.
I'm curious : why "kibana's usage of numeral.js" is "a big problem" ?
I mean : numeral.js is a lib which answers perfectly number formatting.

@epixa
Copy link
Contributor

epixa commented Jul 18, 2018

@fbaligand There are three reasons, primarily:

  1. We're not using numeral.js, we're using a custom fork of it that is considerably behind and to date we don't have a good plan for reconciling.
  2. The numeral.js library itself, despite being more up to date than our own fork, is apparently abandoned. That's a risky situation for a library focused on string parsing and formatting.
  3. Even if we fixed the forking problem and numeral.js development started back up again, we're still exposing the raw API of a third party dependency as a feature of Kibana. A regression or intentional backwards compatibility break in that library could break Kibana for people using those features, and we wouldn't know about it until after it shipped. We could also be put into a situation where an important security fix exists but only in a newer major version that requires us to introduce breaking changes in Kibana in a patch version.

@fbaligand
Copy link
Contributor

fbaligand commented Jul 22, 2018

Hi @epixa ,

Thanks for your detailed answer.
I can clearly understand your first two reasons.
Considering the third, well, that's software life. We benefit from dependencies features, as we suffer from its flaws.
BTW, in the same way, Kibana is exposing Lucene query language and moment.js format to users.

That said, what is the plan to eventually replace numeraljs ? Is there another library as far as powerful, and well-maintained ? Or do you plan an internal implementation ?
Up to me, the better plan would be to integrate @spalger fork into Kibana-core code, with some refactoring. It would allow to keep powerful numeraljs syntax and features (like locale support).

@spalger
Copy link
Contributor

spalger commented Jul 23, 2018

I think it's still really risky to modify numeral.js because it is not very strict with its formats; nearly any combination of letters, numbers and symbols creates a "valid" format. This makes it nearly impossible to understand the potential scope or impact of changes to numeral internals. Because of this I'm convinced that any meaningful improvement to numeral.js will likely mess up the way data is represented for some number of users, which could have serious repercussions in many scenarios.

The current fork as has pretty decent test coverage, but I think it could use a lot more, and that the format syntax either needs a formal specification or to be replaced with a more restrictive API that we can fully understand.

Because of this I would prefer that we approach this the way we are approaching our dependency on the Lucene query syntax. With KQL/kuery we started from scratch, implementing features and syntax in a way that Kibana fully understands so that we have full control over the syntax and how it impacts Kibana users.

Applying this strategy to number formatting, we would continue supporting numeral.js formats for a while, but at the same time create a new solution that we fully understand. We would likely be able to automatically convert common number formats automatically, and and at some point in the future deprecate and remove numeral.js support, requiring users to manually migrate any unique or unexpected formats to our new number formatting solution.

@fbaligand
Copy link
Contributor

Thanks @spalger for your detailed answer.
It sounds like the reasonable way to slowly replace numeral.js with a more mastered internal implementation.
Anyway, it will be a long way :)

I don't know about the new internal way you want to go. But IMHO, it's very important to keep most of numeral.js syntax expressiveness. It is both simple and powerful, which is clearly its strength, and why numeral.js is so appreciated and used.

@timroes timroes added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure :Management DO NOT USE labels Nov 27, 2018
@jmliu1983
Copy link

Same request from my side, thanks

@boutcher
Copy link

Same request for me, as an attempt to apply color formatting to a double renders this useless 4.99999999999999999999999 ... :(

@timroes timroes added Team:AppArch and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Oct 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Jun 16, 2021
@vadimkibana
Copy link
Contributor

Thank you for contributing to this issue, however, we are closing this issue due to inactivity as part of a backlog grooming effort. If you believe this feature/bug should still be considered, please reopen with a comment.

@vadimkibana vadimkibana closed this as not planned Won't fix, can't repro, duplicate, stale Aug 9, 2022
@fbaligand
Copy link
Contributor

fbaligand commented Aug 9, 2022

Well, up to me, this issue is a bug and should be fixed.
Without that, we can't use "Color" feature for decimal numbers as they are displayed with a bunch of decimals.
By the way, there are 5 stars on the issue, so this is of interest to several people.

So please re-open.

alexwizp added a commit to alexwizp/kibana that referenced this issue Feb 20, 2024
alexwizp added a commit to alexwizp/kibana that referenced this issue Feb 20, 2024
alexwizp added a commit to alexwizp/kibana that referenced this issue Feb 21, 2024
alexwizp added a commit to alexwizp/kibana that referenced this issue Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:FieldFormatters Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:enhancement
Projects
None yet
Development

No branches or pull requests