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

Comma separtor of four-digit number on axes is not consistent #841

Closed
satotake opened this issue Aug 11, 2016 · 4 comments
Closed

Comma separtor of four-digit number on axes is not consistent #841

satotake opened this issue Aug 11, 2016 · 4 comments

Comments

@satotake
Copy link
Contributor

satotake commented Aug 11, 2016

Codepen

http://codepen.io/satotake/pen/wWRpbd

Condition

  • Yaxis labels contains any 4-digit integers such as 3000
  • and layout.yaxis.exponentformat = "none"

Unexpected output

  • 4-digit integers on axes has no-comma separation
    • It is expected that value 3000 is displayed as 3,000
    • (on the other hand, more than 5-digit integers display as expected 30000 >>>> 30,000 )

Cause

  • Spec of lib.numSeparate
  • numSeparate is used in various contexts such as hover, xaxis, yaxis and so on
  • numSeparate does not add comma separation to 4-digit int intentionally considering about Year value
    • in order to avoid to convert 2016 into 2,016

Suggestion

How do you think about addition of the optional flag to numSeparation ?
Which enable us to discriminate between year values or not.

@etpinard
Copy link
Contributor

Thanks very much for this very detailed report. All your points are correct ✅

Adding this feature as an option sounds like the way to go here.

We could either add a character to layout.seperators corresponding to the 4-digit thousand separator. For example

layout.separators = '.,'; 
// gives
// 3000 -> 3000
// 30000 -> 30,000

layout.separators = '.,,';
// gives
// 3000 -> 3,000
// 30000 -> 30,000

where the missing layout.separators[2] character in in the first case corresponds to no separator at all in 4-digit integers.

Or, we could add another attribute. a boolean flag e.g. separatethousands will be the easiest. Or, maybe we could try to add in something more general like integer attribute firstseparator with default 4 for example:

layout.firstseparator = 3;
// gives
// 3000 -> 3,000
// 30000 -> 30,000

layout.firstseparator = 5;
// gives
// 3000 -> 30000
// 30000 -> 3,0000

... hmm maybe that last example isn't worth supporting. Just a thought.

@satotake What are your thoughts?

@satotake
Copy link
Contributor Author

satotake commented Aug 12, 2016

Thank you for your suggenstions, @etpinard !

It may not be friendly for strangers to set properties like layout.separators = '.,,';
And, The style like layout.firstseparator = 5; is too much at least for me.

it sounds good for me to add a boolean flag like separatethousands = true;. It is very simple.

In addition to this specification, the bool flag should be separated by contexts in my opinion.
e.g.)

layouts.xaxis.separatethousands = true;
layouts.yaxis.separatethousands = false;
...

@satotake satotake changed the title Comma separtor of four-digit number on axes are not consistent Comma separtor of four-digit number on axes is not consistent Aug 12, 2016
@satotake
Copy link
Contributor Author

@etpinard
I made PR (#848) for this issue!
Please review it.

@etpinard
Copy link
Contributor

done in #848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants