-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Metric vis should display a ?
or 0 for empty sets
#5532
Conversation
…clude metric vis from showing no results. The metric vis should fall through to its own controller.
… 0 or ? for empty sets where appropropriate.
…and cleaning up code in the metric vis controller.
…that null and NaN values can be returned and needed to be handled. I re-organized the code to handle null, undefined, and NaN values in the metric controller.
@tsullivan I fixed the issues, assigning it back to you. |
@@ -8,13 +9,22 @@ define(function (require) { | |||
|
|||
var metrics = $scope.metrics = []; | |||
|
|||
$scope.isNullorNaN = function (val) { | |||
return _.isNull(val) || isNaN(val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to _.isNaN(val)
? The global isNaN
is not a good part of JS, it has weird behavior for non-numeric arguments.
@tsullivan made the corrections, passing back to you. |
LGTM |
@@ -8,13 +9,22 @@ define(function (require) { | |||
|
|||
var metrics = $scope.metrics = []; | |||
|
|||
$scope.isNullorNaN = function (val) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this attached to $scope? I don't see it used in any views or child scopes.
@@ -44,6 +44,19 @@ define(function (require) { | |||
var getVisEl = getter('.visualize-chart'); | |||
var getVisContainer = getter('.vis-container'); | |||
|
|||
// Show no results message when hasZeroHits is true and it requires search | |||
$scope.showNoResultsMessage = function () { | |||
var requiresSearch = _.get($scope, 'vis.type.requiresSearch'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can _.has be used for requireSearch and shouldNotShowMessage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, I didn't notice these were both already booleans. it looks like the has check works fine but probably wasn't needed
@jbudz I made changes based on your comments. Passing it back to you. |
LGTM |
Metric vis should display a `?` or 0 for empty sets
Backported in #5877 |
I am not sure if 0 (zero) is a correct default value for SUM(of "no results"). If your data domain includes negative numbers, then zero is as good a number as fourteen. Maybe in that case, a N/A or something is more appropriate? Just saying... |
I am using Kibana 5.5.1 and I do get the "No results found :-| " message for a simple count aggregation when there are no records. I really would expect a "0" there, and reading the above discussion, it sounds like "0" is what it should be doing... |
Also seeing "No results found" message in 5.5.2. Regression or do I have to switch something "on?" |
I'm using 5.2.2, I still get "No results found" prompt. Is there any special configuration? |
no results found actually indicates there were no records in the time range you are looking at (i think) |
Closes #3682.
In metric visualizations, when no data are available, a
no results
message should not be displayed. In the case of an empty (data) set, forCount
,Unique Count
, andSum
, 0 should be displayed. For all other metrics, a ? should be displayed.