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.

Sign up for GitHub

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

Jump to bottom

Update dependency @elastic/charts to v19.1.2 #64759

Merged
merged 10 commits into from
May 4, 2020
Merged

Update dependency @elastic/charts to v19.1.2 #64759

merged 10 commits into from
May 4, 2020

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Apr 29, 2020 •
edited
Loading

Summary

This PR contains the following updates:

Package Type Update Change
@elastic/charts dependencies major 18.4.2 -> 19.1.2

Release Notes

19.1.2 (2020-05-04)

Bug Fixes

19.1.1 (2020-04-30)

Bug Fixes

19.1.0 (2020-04-30)

Features

  • partition: treemap group text in grooves (#652) (304dd48)

19.0.0 (2020-04-28)

Bug Fixes

Features

BREAKING CHANGES

  • brush: The type used by the BrushEndListener is now in the following form
{
  x?: [number, number];
  y?: Array<{ groupId: GroupId; values: [number, number];}>
}

where x contains an array of [min, max] values, and the y property is an optional array of objects, containing the GroupId and the values of the brush for that specific axis.

  • annotations: In the rectangular annotation, the y coordinates are now inverted: the y0 parameter of the coordinates now refers to the minimum value and the y1 value refers to the maximum value of the y domain.

@elastic/charts update procedure

@markov00 markov00 added Feature:ElasticCharts Issues related to the elastic-charts library release_note:skip Skip the PR/issue when compiling release notes dependencies Pull requests that update a dependency file labels Apr 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/datavis (Feature:ElasticCharts)

@markov00 markov00 mentioned this pull request Apr 29, 2020
Closed
@markov00 markov00 marked this pull request as ready for review April 30, 2020 10:48
@markov00 markov00 requested a review from a team as a code owner April 30, 2020 10:48
@markov00 markov00 requested a review from a team April 30, 2020 10:48
@markov00 markov00 requested review from a team as code owners April 30, 2020 10:48
@markov00 markov00 mentioned this pull request Apr 30, 2020
Closed
1 task
@FrankHassanabad
Copy link
Contributor

Gave this a test run within Elastic Security and am seeing this stack trace which is not present on master at the moment when you zoom in within a chart:

error_charts_zoom

Full stack trace:

Uncaught TypeError: Cannot read property 'xDomain' of undefined
    at compute_series_domains.js:14
    at resultFuncWithRecomputations (index.js:77)
    at index.js:70
    at index.js:30
    at index.js:84
    at index.js:30
    at selector (index.js:105)
    at index.js:80
    at index.js:30
    at Object.selector [as computeLegendSelector] (index.js:105)
(anonymous)	@	compute_series_domains.js:14
resultFuncWithRecomputations	@	index.js:77
(anonymous)	@	index.js:70
(anonymous)	@	index.js:30
(anonymous)	@	index.js:84
(anonymous)	@	index.js:30
selector	@	index.js:105
(anonymous)	@	index.js:80
(anonymous)	@	index.js:30
selector	@	index.js:105
push.../../node_modules/@elastic/charts/dist/chart_types/xy_chart/state/chart_state.js.XYAxisChartState.getLegendItems	@	chart_state.js:53
push.../../node_modules/@elastic/charts/dist/state/selectors/get_legend_items.js.exports.getLegendItemsSelector	@	get_legend_items.js:6
(anonymous)	@	chart_state.js:127
p	@	VM1019:1
v	@	VM1019:1
(anonymous)	@	VM1019:1
dispatch	@	redux.js:212
e	@	VM1019:1
dispatch	@	VM1019:1
(anonymous)	@	redux.js:475
(anonymous)	@	chart_container.js:86
requestAnimationFrame (async)		
ChartContainerComponent._this.handleBrushEnd	@	chart_container.js:85

spalger
spalger approved these changes Apr 30, 2020
View reviewed changes
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Operations: kbn-ui-shared-deps LGTM

@markov00 markov00 mentioned this pull request Apr 30, 2020
Merged
1 task
@wylieconlon
Copy link
Contributor

Lens just merged our brushing function, I think this PR needs to be updated: #62636

@markov00
Copy link
Member Author

markov00 commented Apr 30, 2020 •
edited
Loading

@FrankHassanabad I've fixed the issue on @elastic/charts and updated the PR with the fix.

@wylieconlon I've merged master and fixed the breaking changes in Lens here: 7049d2e

@markov00 markov00 changed the title Update dependency @elastic/charts to v19.0.0 Update dependency @elastic/charts to v19.1.1 Apr 30, 2020
wylieconlon
wylieconlon approved these changes Apr 30, 2020
View reviewed changes
Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

LGTM, tested drag interactions in Lens, Discover and TSVB

@Kerry350 Kerry350 self-requested a review May 4, 2020 12:48
FrankHassanabad
FrankHassanabad approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Checked out the branch and tested:

  • Used the drill downs from zoom in within hosts
  • Used the mini histogram viewer and tested that it still operates and that zoom in works
  • Tested the network page to ensure the charting works there.
  • Ensured there were no console errors and no server side errors when doing the tests

👍 , Thanks for the fast fixes for anything found here. Appreciate it!

angorayc
angorayc approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@angorayc angorayc left a comment

Choose a reason for hiding this comment

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

Approving changes for SIEM! Thank you @markov00

jasonrhodes
jasonrhodes approved these changes May 4, 2020
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Logs and metrics changes LGTM

justinkambic
justinkambic approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM - all Uptime charts seem to be functioning normally

@markov00 markov00 changed the title Update dependency @elastic/charts to v19.1.1 Update dependency @elastic/charts to v19.1.2 May 4, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@nickofthyme nickofthyme merged commit 2d4dc80 into elastic:master May 4, 2020
@markov00 markov00 deleted the 2020_09_04-upgrade-ech-19-0-0 branch May 5, 2020 08:22
@markov00 markov00 mentioned this pull request May 5, 2020
Merged
markov00 added a commit to markov00/kibana that referenced this pull request May 5, 2020
@markov00
Update dependency @elastic/charts to v19.1.2 (elastic#64759)
707174c 
* build: upgrade @elastic/charts to 19.0.0

* refactor: onBrushEnd breaking changes

* fix: missing onBrushEnd argument changes

* fix: update ech to 19.1.1

* fix: lens onBrushEnd breaking changes

* update to 19.1.2
# Conflicts:
#	package.json
markov00 added a commit that referenced this pull request May 5, 2020
@markov00
Update dependency @elastic/charts to v19.1.2 (#64759) (#65235)
e81ff9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@wylieconlon wylieconlon wylieconlon approved these changes

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

@spalger spalger spalger approved these changes

@angorayc angorayc angorayc approved these changes

@justinkambic justinkambic justinkambic approved these changes

@Kerry350 Kerry350 Awaiting requested review from Kerry350

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file Feature:ElasticCharts Issues related to the elastic-charts library release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
10 participants
@markov00 @elasticmachine @FrankHassanabad @wylieconlon @kibanamachine @jasonrhodes @spalger @angorayc @justinkambic @nickofthyme