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

Fix histogram zr handler #709

Merged
merged 3 commits into from
Jun 14, 2023
Merged

Fix histogram zr handler #709

merged 3 commits into from
Jun 14, 2023

Conversation

VictorVelarde
Copy link
Contributor

@VictorVelarde VictorVelarde commented Jun 14, 2023

Description

Shortcut: 322439
[sc-322439]

HistogramWidget fix, broken after adding skeleton (missing ref in object when dealing with echart events, through ZRender lib)

Type of change

  • Fix

Acceptance

HistogramWidget doesn't break. You can move around the map and it correctly manages skeleton on-off

Basic checklist

  • Good PR name
  • Shortcut link
  • Changelog entry
  • Just one issue per PR
  • GitHub labels
  • Proper status & reviewers
  • Tests
  • Documentation

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #322439: Histogram widget crashes.

@VictorVelarde VictorVelarde force-pushed the fix-histogram-zr-handler branch from 453a424 to 8cf4f3b Compare June 14, 2023 17:51
@@ -242,7 +242,7 @@ export default function useTimeSeriesInteractivity({ echartsInstance, data }) {

// Aux
function addEventWithCleanUp(zr, eventKey, event) {
if (zr) {
if (zr && zr.handler) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Preventive

@@ -114,7 +114,7 @@ export default function useHistogramInteractivity({

// Aux
function addEventWithCleanUp(zr, eventKey, event) {
if (zr) {
if (zr && zr.handler) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key part here

@coveralls
Copy link
Collaborator

coveralls commented Jun 14, 2023

Pull Request Test Coverage Report for Build 5270475680

  • 4 of 7 (57.14%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 71.399%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/react-ui/src/widgets/HistogramWidgetUI/useHistogramInteractivity.js 3 4 75.0%
packages/react-ui/src/widgets/TimeSeriesWidgetUI/hooks/useTimeSeriesInteractivity.js 1 3 33.33%
Totals Coverage Status
Change from base Build 5269045523: -0.04%
Covered Lines: 2210
Relevant Lines: 2861

💛 - Coveralls

@VictorVelarde VictorVelarde merged commit 999e81c into master Jun 14, 2023
@VictorVelarde VictorVelarde deleted the fix-histogram-zr-handler branch June 14, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants