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

Add condition to deleting marker floaters logic #504

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

jernestmyers
Copy link
Contributor

Resolves #427

Previously, deleting a floater always invoked setActiveVisualizationId(undefined). Now, we'll only do so if the viz being deleted is the active viz.

NOTE
The deletion will update analysisState, causing the loader to spin on the active viz. This may be another thing to consider when we have our deep think about our data requests. As a user, I'd be a bit confused why deleting a non-active viz results in a loading state. This issue feels a bit out of scope for this ticket, but perhaps it's not? Thoughts on if I should investigate further in the context of this ticket, or should it wait until our proper deep think?

Copy link
Member

@dmfalke dmfalke left a comment

Choose a reason for hiding this comment

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

This is good. Regarding the spinner, I didn't see it in my test. I had a timeseries plot open when I deleted a line plot. Maybe it depends on the type of the open viz?

@jernestmyers jernestmyers merged commit 8d693c8 into main Sep 15, 2023
1 check passed
@jernestmyers jernestmyers deleted the 427-deleting-viz-floaters-bug branch September 15, 2023 13:17
@bobular
Copy link
Member

bobular commented Sep 16, 2023

I agree we should review the data call dependency logic.

I'll see if I can reproduce the spinner thing later/next week.

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.

Map - deleting a floating plot from the supporting plots list has unwanted side effects
3 participants