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

Fixes issue #1658: Field formatter trying to apply missing formatter #1753

Merged
merged 4 commits into from
Oct 28, 2014

Conversation

stormpython
Copy link
Contributor

This pull requests closes #1658.

The code that used the field format functions from the raw object has been removed. Fixing the current bug will cause values in the legend for Bar, Line, and Area charts to be broken for time values. Instead of displaying human readable date formats, timestamps will be displayed instead. We will need to refactor the histogram data converter to send either label formatters or change the current xAxisFormatter to make it more general for any value type. For more details, see my comments in #1658.

I have also updated the formatters for the pie chart. The pie chart data converter sends field formatters for all of its data values, which means the pie legend values will be formatted appropriately.

})
.attr('fill', function (d) {
return color(d.label);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

The added indentation here seems inconsistent with how it is applied elsewhere in the code. Was this an intentional change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rashidkpc Nope, sometimes my editor adds indents automatically. Fixing.

@rashidkpc
Copy link
Contributor

Other than that minor nitpick, this LGTM

rashidkpc added a commit that referenced this pull request Oct 28, 2014
Fixes issue #1658: Field formatter trying to apply missing formatter
@rashidkpc rashidkpc merged commit faa64ec into elastic:master Oct 28, 2014
@stormpython stormpython deleted the feature/fix_1658 branch November 4, 2014 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Field formatter trying to apply missing formatter
2 participants