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

TimeSeriesWidget: support removing series in mounted widget #863

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

zbigg
Copy link
Contributor

@zbigg zbigg commented Apr 18, 2024

Description

Shortcut: https://app.shortcut.com/cartoteam/story/404044/monom-solutions-sl-time-series-widget-not-updating-when-parameters-change

TimeseriesWidget: Pass notMerge to ReactEcharts so it doens't attempt to merge current options with old ones. This effectively allows removing series from widget instance.

Background:
Eacharts/react-echarts by default merge objects passed in setOptions so it's not possible to remove old series with default props update mechanism. For fully state-less, reactive behavior "setOption" strategy must changed to notMerge so no previous state lingers in echarts component instance.

Ref:

Type of change

  • Fix

Acceptance

Create setup where series may disappear automatically. For example through data-source filtering and configure splitByCategory on same column as filtering.

  • remove series one by one.
  • series should disappear from a chart as they are dissapearing from legend

Basic checklist

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

@zbigg zbigg assigned padawannn and donmccurdy and unassigned donmccurdy and padawannn Apr 18, 2024
@zbigg zbigg changed the title TimeSeries: allow removing series TimeSeriesWidget: allow removing series Apr 18, 2024
@zbigg zbigg changed the title TimeSeriesWidget: allow removing series TimeSeriesWidget: support removing series in mounted wirget Apr 18, 2024
@zbigg zbigg changed the title TimeSeriesWidget: support removing series in mounted wirget TimeSeriesWidget: support removing series in mounted widget Apr 18, 2024
Copy link

github-actions bot commented Apr 18, 2024

Pull Request Test Coverage Report for Build 8736752044

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.233%

Totals Coverage Status
Change from base Build 8704914693: 0.0%
Covered Lines: 2797
Relevant Lines: 3617

💛 - Coveralls

@zbigg zbigg added bug Something isn't working widgets labels Apr 18, 2024
Copy link

github-actions bot commented Apr 18, 2024

Visit the preview URL for this PR (updated for commit 6686eca):

https://cartodb-fb-storybook-react-dev--pr863-bug-sc-404044-mo-9if9u372.web.app

(expires Thu, 25 Apr 2024 11:06:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c

@zbigg zbigg changed the base branch from master to 2.5-release April 18, 2024 10:48
@zbigg zbigg changed the base branch from 2.5-release to master April 18, 2024 11:00
@zbigg zbigg force-pushed the bug/sc-404044/monom-solutions-sl-time-series-widget-not branch from f406bb2 to 6686eca Compare April 18, 2024 11:01
@zbigg zbigg merged commit 520d255 into master Apr 18, 2024
2 checks passed
@zbigg zbigg mentioned this pull request Apr 18, 2024
zbigg added a commit that referenced this pull request Apr 18, 2024
* TimeSeriesWidget: support removing series in mounted widget (#863)

* pre-release changelog update

* v2.5.3
@zbigg zbigg deleted the bug/sc-404044/monom-solutions-sl-time-series-widget-not branch April 18, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants