Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Fix issues with disabled plot series #7342

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

jtbandes
Copy link
Member

@jtbandes jtbandes commented Jan 18, 2024

User-Facing Changes
Fixed issues with disabled series in the Plot panel.

Description

  • Add PlotCoordinator.destroy() to fix an issue where switching from Timestamp to Index mode would still show the timestamp data (because the old coordinator rendered after the new one)
  • Ensure all builders include an empty dataset in getViewportDatasets for disabled series
  • Ensure all builders keep track of the new enabled setting in setSeries (even if the series did not otherwise change)

Copy link
Contributor

@defunctzombie defunctzombie left a comment

Choose a reason for hiding this comment

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

We need to destroy the plot coordinator because it is doing async work that might complete after we've setup a different coordinator? Should we be aborting that work? That might be harder to do than the booleans you've added.

@jtbandes
Copy link
Member Author

I’m not sure the aborting is so important - I suppose it would be the most thorough thing to do but I didn’t feel compelled.

@jtbandes jtbandes merged commit 5b57540 into main Jan 19, 2024
13 of 14 checks passed
@jtbandes jtbandes deleted the jacob/fg-6340-tooltip-disabled-series-fix branch January 19, 2024 03:49
jtbandes added a commit that referenced this pull request Jan 22, 2024
**User-Facing Changes**
Not worth calling out

**Description**
Fixed bugs with series that are invalid (fail to parse) causing tooltips
and current values to display on a different series. (Similar to #7342
but for invalid series)

Approach: instead of each builder handling its own `data: []` defaults
(added in #7342), the builders use the series' `configIndex` to output
`datasetsByConfigIndex`. The coordinator fills in missing entries with
`data: []` before passing to the renderer.


https://github.com/foxglove/studio/assets/14237/c8f6ba28-8eb9-46d8-a3cc-6b914c7aab93
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants