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

remove 'No results found' treatment for single count visualization #5092

Conversation

tsullivan
Copy link
Member

Addresses: #3682 (count and unique count widget)

screen shot 2015-10-12 at 3 12 50 pm

@tbragin
Copy link
Contributor

tbragin commented Oct 9, 2015

Addresses issue #3682

LGTM, functionally

@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch from 593d100 to 3db8819 Compare October 9, 2015 23:42
@tsullivan tsullivan assigned tsullivan and lukasolson and unassigned tsullivan Oct 10, 2015
@lukasolson
Copy link
Member

Yeah, unfortunately, this has a lot of side effects. One example is the table vis type, which is completely empty if there are no results. Also, date histograms still show "no results found."

I think we should try to make a change that's limited in scope to modifying the metric vis to show "0" when there are no results, and only when the metric agg is set to "count" or "unique count".

@lukasolson lukasolson assigned tsullivan and unassigned lukasolson Oct 12, 2015
@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch from 3db8819 to 87c1e9f Compare October 12, 2015 22:14
@tsullivan
Copy link
Member Author

@lukasolson I updated the PR and the screenshots. Now I'm having only the metric vis type modified (table and date histogram vis are untouched). It's shows a "no data available" message if it encounters something with no readable value.

Assuming I am on the right track now, before this is merged, I'd like to discuss how to remove some repetition of code from visualize.html - I'd like to have a single flag to check in the scope that holds the value of vis.type.requiresSearch && esResp.hits.total === 0 && vis.type.name !== 'metric'

@tsullivan tsullivan assigned lukasolson and unassigned tsullivan Oct 12, 2015
@lukasolson
Copy link
Member

Yeah, this looks good, and I agree that it makes sense to have a value in scope instead of checking all three of those values each time.

LGTM.

@lukasolson lukasolson assigned tsullivan and unassigned lukasolson Oct 13, 2015
@rashidkpc rashidkpc added v4.4.0 and removed v4.3.0 labels Oct 26, 2015
@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch from 87c1e9f to 69c9048 Compare October 30, 2015 23:09
@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch 3 times, most recently from 68031e5 to a483150 Compare November 3, 2015 23:46
@tsullivan tsullivan assigned lukasolson and unassigned tsullivan Nov 3, 2015
@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch from a483150 to 28df328 Compare November 4, 2015 00:19
@tsullivan tsullivan assigned tsullivan and unassigned lukasolson Nov 4, 2015
@tsullivan tsullivan force-pushed the fix-no-results-found-in-single-value-widget branch from 28df328 to 586720e Compare November 4, 2015 16:38
@tsullivan
Copy link
Member Author

After rebasing this branch, the feature is not testing out as well as it used to. It has new buggy behavior where it'll throw an error for certain kinds of metric aggregations if the result count is 0.

@tsullivan
Copy link
Member Author

Closing this for the same reason as #5253. It's not such a low fruit, it needs more attention

@tsullivan tsullivan closed this Nov 19, 2015
@tsullivan tsullivan deleted the fix-no-results-found-in-single-value-widget branch July 19, 2018 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants