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 in formatters. #533

Merged
merged 11 commits into from
Jun 12, 2014
Merged

Add in formatters. #533

merged 11 commits into from
Jun 12, 2014

Conversation

jtlan
Copy link
Contributor

@jtlan jtlan commented Jun 9, 2014

Plottable.Util.Formatters now includes functions that generate formatters:

  • identity() : String-ifies the input.
  • general() : Shows at most [precision] decimal places (using round-half-up).
  • fixed() : Shows exactly [precision] decimal places (IEEE 754 rounding).
  • currency() : Displays fixed() values with a currency symbol (defaults to $)
  • percentage() : Dipplayes fixed() values as percentages.

Justin Lan added 2 commits June 9, 2014 16:26
Plottable.Util.Formatters now includes functions that generate formatters:

- identity() : String-ifies the input.
- general() : Shows at most [precision] decimal places (using round-half-up).
- fixed() : Shows exactly [precision] decimal places (IEEE 754 rounding).
- currency() : Displays fixed() values with a currency symbol (defaults to $)
- percentage() : Dipplayes fixed() values as percentages.
export interface IFormatter {
(d: any): String;
}
export module Formatters {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt be plural. maybe should just be Format not Formatter?

Also fix breaking test due to character-encoding problems.
@crmorford
Copy link
Contributor

percentage not working as expected http://jsfiddle.net/cmorford/LMXct/

@crmorford
Copy link
Contributor

percentage(3) breaks yaxis
http://jsfiddle.net/cmorford/T63rz/

ditto for fixed() at higher numbers
http://jsfiddle.net/cmorford/8DheP/2/

@crmorford
Copy link
Contributor

I know that something Slate wanted was for values to only show up once if there were duplicates. So if you use fixed(0), you should only see one 0, one 1, one 2, etc...

Is this the branch to fix that in?
http://jsfiddle.net/cmorford/8DheP/3/

@jtlan
Copy link
Contributor Author

jtlan commented Jun 10, 2014

re: percentage(3) and fixed(6):
The labels get too long and are hidden as a result. The upcoming NumberAxis should fix this. In the meantime, setting the width of the YAxis to a larger number with width() will display the tick labels.

Justin Lan added 2 commits June 10, 2014 17:18
Formatters now also have an option to show only values that are unchanged by
formatting. For example:

var f = new Plottable.Util.Formatter.Fixed(3);
f.format(1); // "1.000"
f.format(1.234); // "1.234"
f.format(1.2345); // ""

This value can be set with .showOnlyUnchangedValues() and is true by default.

In addition, added in Formatter.Custom to allow the Javascript consumers of
Plottable to create new Formatters without writing TypeScript code.
@crmorford
Copy link
Contributor

Not a big fan of this behavior on the y-axis
http://jsfiddle.net/cmorford/LMXct/1/
But that's a different problem, right?

@crmorford
Copy link
Contributor

Looks good!
This is a fun one to play with with different formatters
http://jsfiddle.net/cmorford/79u7g/

@jtlan
Copy link
Contributor Author

jtlan commented Jun 11, 2014

That's a cutoff problem, yeah. Try increasing the width.

Conflicts:
	quicktests/gridlines-quicktest.html
@crmorford
Copy link
Contributor

GIL

if (typeof d === "number") {
if (Math.abs(d) < 1) {
return String(Math.round(1000 * d) / 1000); // round to 3 decimal places
}
return numberFormatter(d);
}
return d;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to factor this base format function into a defaultFormatter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered having it in the general formatter, but I'm thinking formatters should take Numbers going forward, instead of anys. However, that doesn't mesh well with the inevitable need for formatters for DateAxis..

@teamdandelion
Copy link
Contributor

minor change then GIL

Also added tests for the Identity formatter.
@crmorford
Copy link
Contributor

I do think it's odd that only Identity and General are defined for OrdinalScale+(X/Y)Axis, I'd prefer all or nothing.

How soon are those getting deprecated?

@jtlan
Copy link
Contributor Author

jtlan commented Jun 11, 2014

@danmane Can you explain what you mean in more detail?

jtlan added a commit that referenced this pull request Jun 12, 2014
@jtlan jtlan merged commit 91979bf into master Jun 12, 2014
@jtlan jtlan deleted the formatters branch June 12, 2014 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants