Skip to content

fix for multiple plots that consist of a single plot#210

Merged
vandenman merged 1 commit into
jasp-stats:masterfrom
vandenman:fix/single_multiplots
Jul 8, 2026
Merged

fix for multiple plots that consist of a single plot#210
vandenman merged 1 commit into
jasp-stats:masterfrom
vandenman:fix/single_multiplots

Conversation

@vandenman

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts editImage()’s interactive editing path to support interactive plot objects that may be collections (e.g., “multiple plots that consist of a single plot”), by cloning and comparing plots differently depending on whether the object is a plain ggplot or a multi-plot container.

Changes:

  • Broadened the "interactive" editing branch to handle non-ggplot plot objects by cloning the container and cloning each subplot.
  • Reworked the “did editing change anything?” check to support multi-plot objects rather than relying solely on identical() for ggplots.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread R/common.R
Comment thread R/common.R
Comment thread R/common.R
Comment on lines +1131 to +1133
} else {
!all(vapply(seq_along(plot), \(i) identical(plot[[i]], newPlot[[i]]), FUN.VALUE = logical(1L)))
}
@juliuspfadt

Copy link
Copy Markdown

what do you think of the comments?

@juliuspfadt juliuspfadt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

see above.

@vandenman

Copy link
Copy Markdown
Contributor Author

eh here I think the AI is overdoing it. The code is written generically so that when we can actually edit multiple panels in the future then it still works (not entirely, but mostly). But to already now built in code that work when we add or remove subplots is a bit too much.

@juliuspfadt juliuspfadt self-requested a review July 8, 2026 09:03

@juliuspfadt juliuspfadt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great!

@vandenman vandenman merged commit fe5fe0c into jasp-stats:master Jul 8, 2026
8 of 9 checks passed
@vandenman vandenman deleted the fix/single_multiplots branch July 8, 2026 12:50
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.

3 participants