Fix no auto_bounds preventing pan/zoom/scroll #218
Conversation
The previous logic would force reset the bounds for an axis if its auto_bounds was false. I believe that the behavior of mem.auto_bounds already sufficiently achieves the intended behavior here. Scenarios tested: - User can pan/zoom/scroll with auto_bounds true/false - Double-click resets auto_bounds the plot (regardless of default_auto_bounds setting) - Plot bounds are updated when plot data changes after double-clicking - User can still interact with the plot (pan/zoom/scroll) after double-clicking Fixes emilk#151
|
View snapshot changes at kitdiff |
kitizz
left a comment
kitizz
left a comment
There was a problem hiding this comment.
I'm going to write a test case for this. Not seeing any examples for non-screenshot based tests in this repo. Just need to go and figure out how to set up a Context and Ui for testing...
| } | ||
|
|
||
| // Reset bounds to initial bounds if they haven't been modified. | ||
| if (!self.default_auto_bounds.x && !any_dynamic_modifications) || mem.auto_bounds.x { |
There was a problem hiding this comment.
I hope my understanding of the full intention here is correct. I thought long and hard about it, and played around with a number of different configurations, and this works well.
But I'm suspicious that I'm missing something important
There was a problem hiding this comment.
I could not find any problems here either
|
Writing tests is blocked on some missing features in (See #219) |
bircni
left a comment
bircni
left a comment
There was a problem hiding this comment.
lgtm
Lets get this in prod and write an issue to write test once the kittest is added
| } | ||
|
|
||
| // Reset bounds to initial bounds if they haven't been modified. | ||
| if (!self.default_auto_bounds.x && !any_dynamic_modifications) || mem.auto_bounds.x { |
There was a problem hiding this comment.
I could not find any problems here either
|
Good job, thank you! I did not find any issues either. Agreed we need non-screenshot tests, and thank you so much for taking an initiative in this direction, it's exactly what is needed here! |
michalsustr
added
the
include in changelog
3 participants
The previous logic would force reset the bounds for an axis if its auto_bounds was false. I believe that the behavior of mem.auto_bounds already sufficiently achieves the intended behavior here.
Scenarios tested: