Merged
Conversation
Contributor
|
LGTM but I will wait for @doutriaux1 to review since I am not sure if this could open some other issues. |
Contributor
Author
|
Sounds good. It should only affect the interactive code; the "Save Template" and "Reset Template" buttons in the Configure Menu in particular, but I'm happy to wait till @doutriaux1 has time to check this out on Monday.
|
Contributor
|
LGTM as well, it's much cleaner like this too! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, when you resize a plot that has a ratio in
interact()mode, things go badly.Here's how the plot looks when you first make it (Perfectly correct)

Here's how it looks when you shrink the window (Not too bad...)

But hey, let's compare that to another plot that was actually plotted at that window size, for giggles...
Here's the x canvas' plot:

Here's the y canvas' plot:

While we can debate whether the labels should run over each other like that, the second behavior is the "correct" behavior, as the resizing in the first window only happens while in interact mode.
This PR should fix this issue– I looked into it, and plots given a ratio actually already generate a new template. My system was snagging that copied template, and then using it as the basis for a new template. This would cause some cascading error in the resizing logic, which is why the resize came out weird. Since the template is already a copy, I can just add the original and the copy into the existing store of templates. As a happy side effect, the code got a lot shorter and less wacky.
@doutriaux1 @aashish24 Mind taking a look?