[XY Charts] fix partial histogram endzones annotations by nickofthyme · Pull Request #93091 · elastic/kibana

nickofthyme · 2021-03-01T19:17:13Z

Summary

Partial buckets were not being rendered on the xy charts due to the TimefilterContract.getActiveBounds method returning undefined. Changing this call to use the getBounds method instead, solves the issue.

Screen.Recording.2021-03-01.at.01.11.38.PM.mp4

Checklist

Unit or functional tests were updated or added to match the most common scenarios

nickofthyme · 2021-03-01T19:27:03Z

@flash1293 Do you know if anything changed between getActiveBounds and getBounds recently? It doesn't look to me like either has changed recently.

I'm pretty sure this was working as of #78154. It looks like vislib uses getBounds on the TimeBuckets agg for this calulcation.

kibana/src/plugins/vis_type_vislib/public/to_ast.ts

Line 53 in 1a77095

(dimensions.x.params as DateHistogramParams).bounds = xAgg.buckets.getBounds();

In any case, I think this is a correct fix.

markov00 · 2021-03-02T14:17:26Z

src/plugins/vis_type_xy/public/utils/domain.ts


  if ((params as DateHistogramParams).date) {
-    const bounds = getTimefilter().getActiveBounds();
+    const bounds = getTimefilter().getBounds();


@nickofthyme that was changed to use getActiveBounds due to the following:
#89822 (comment)

Oh, so any idea why this would return undefined when a time range is active?

Ok I manually enabled the time filter for date histogram charts. I'm not sure why this is not already happening automatically. But I see timelion is doing this and I modeled the renderer changes after timelion. See https://github.com/elastic/kibana/pull/93091/files

Ok I reverted the code to use xAgg.buckets.getBounds which knows about the enablement of the time filter and is how vislib handles getting the time bounds. Both the time-based and non-time-based data seem to be working fine.

nickofthyme · 2021-03-04T01:38:51Z

jenkins retest this please

elasticmachine · 2021-03-04T13:50:00Z

Pinging @elastic/kibana-app (Team:KibanaApp)

flash1293

Not sure whether that's expected, but if the time filter is not used, partial buckets based on filters are not annotated:

Works fine for time based index patterns. If that's intended for now I will approve the PR.

nickofthyme · 2021-03-04T14:34:43Z

@flash1293 that's a good point, I checked the vislib functionality and it doesn't appear to render the endzone either.

We could just use the bounds of the data instead of the timeFilter. Let me see how difficult that would be to implement.

nickofthyme · 2021-03-04T14:36:52Z

@elasticmachine merge upstream

nickofthyme · 2021-03-04T15:48:41Z

@flash1293 I set the bounds to the data domain in the example above. The endzone logic always annotated the full right side and never the left.

Even so, I think the wording on the prompt in this case would be strange, cuz there's no selected range nor would there be missing data if all data is included.

I think we revisit this at a later time.

flash1293

Makes sense to me, thanks for checking! LGTM

kibanamachine · 2021-03-04T16:28:37Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visTypeXy`	119.9KB	119.8KB	-90.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeXy`	46.5KB	46.3KB	-157.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-03-04T17:09:42Z

💚 Backport successful

✅ 7.12 / #93642
✅ 7.x / #93643

Successful backport PRs will be merged automatically after passing CI.

Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>

* master: (107 commits) [Logs UI] Fix log stream data fetching (elastic#93201) [App Search] Added relevance tuning search preview (elastic#93054) [Canvas] Fix reports embeddables (elastic#93482) [ILM] Added new functional test in ILM for creating a new policy (elastic#92936) Remove direct dependency on statehood package (elastic#93592) [Maps] Track tile loading status (elastic#91585) [Discover][Doc] Improve main documentation (elastic#91854) [Upgrade Assistant] Disable UA and add prompt (elastic#92834) [Snapshot Restore] Remove cloud validation for slm policy (elastic#93609) [Maps] Support GeometryCollections in GeoJson upload (elastic#93507) [XY Charts] fix partial histogram endzones annotations (elastic#93091) [Core] Simplify context typings (elastic#93579) [Alerting] Improving health status check (elastic#93282) [Discover] Restore context documentation (elastic#90784) [core-docs] Edits core-intro section for the new docs system (elastic#93540) add missed codeowners (elastic#89714) fetch node labels via script execution (elastic#93225) [Security Solution] Adds getMockTheme function (elastic#92840) Sort dependencies in package.json correctly (elastic#93590) [Bug] missing timepicker:quickRanges migration (elastic#93409) ...

fix endzones in xy charts

7d4f948

nickofthyme requested review from a team and flash1293 March 1, 2021 19:17

nickofthyme mentioned this pull request Mar 1, 2021

Area chart displays data points on y-axis and user cannot brush on it to see the chart over a large time period #93117

Closed

markov00 reviewed Mar 2, 2021

View reviewed changes

nickofthyme added 3 commits March 2, 2021 11:17

manually enable time filter for date histograms

52b7290

remove disable method

32dc89c

fix bounds to use xaggs bounds not from timefilter

12f6752

nickofthyme linked an issue Mar 2, 2021 that may be closed by this pull request

Area chart displays data points on y-axis and user cannot brush on it to see the chart over a large time period #93117

Closed

nickofthyme added 3 commits March 3, 2021 12:58

Merge branch 'master' into fix-endzones

04f0770

fix failing functional test

9381a7b

fix failing functional tests

542b2ca

flash1293 added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.13.0 labels Mar 4, 2021

flash1293 reviewed Mar 4, 2021

View reviewed changes

Merge branch 'master' into fix-endzones

5b2fbae

flash1293 approved these changes Mar 4, 2021

View reviewed changes

nickofthyme merged commit 667cb14 into elastic:master Mar 4, 2021

nickofthyme deleted the fix-endzones branch March 4, 2021 17:06

This was referenced Mar 4, 2021

[7.x] [XY Charts] fix partial histogram endzones annotations (#93091) #93640

Merged

[7.12] [XY Charts] fix partial histogram endzones annotations (#93091) #93641

Merged

nickofthyme added a commit to nickofthyme/kibana that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (elastic#93091)

b5a46dd

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (elastic#93091)

7c20634

kibanamachine mentioned this pull request Mar 4, 2021

[7.12] [XY Charts] fix partial histogram endzones annotations (#93091) #93642

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (elastic#93091)

eac7345

kibanamachine mentioned this pull request Mar 4, 2021

[7.x] [XY Charts] fix partial histogram endzones annotations (#93091) #93643

Merged

nickofthyme added a commit that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (#93091) (#93640)

3c3e63f

kibanamachine added a commit that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (#93091) (#93643)

2de2474

Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>

kibanamachine added a commit that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (#93091) (#93642)

670fe1a

Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>

nickofthyme added a commit that referenced this pull request Mar 4, 2021

[XY Charts] fix partial histogram endzones annotations (#93091) (#93641)

32e1a52

Conversation

nickofthyme commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

nickofthyme commented Mar 1, 2021

Uh oh!

markov00 Mar 2, 2021

Choose a reason for hiding this comment

Uh oh!

nickofthyme Mar 2, 2021

Choose a reason for hiding this comment

Uh oh!

nickofthyme Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Mar 4, 2021

Uh oh!

elasticmachine commented Mar 4, 2021

Uh oh!

flash1293 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Mar 4, 2021

Uh oh!

nickofthyme commented Mar 4, 2021

Uh oh!

nickofthyme commented Mar 4, 2021

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 4, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

kibanamachine commented Mar 4, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nickofthyme commented Mar 1, 2021 •

edited

Loading

nickofthyme Mar 2, 2021 •

edited

Loading

flash1293 left a comment •

edited

Loading