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

fix: default to changing y axis when changing data aspect #2087

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

saecki
Copy link
Contributor

@saecki saecki commented Sep 26, 2022

Closes #2077.

@cessen @DusterTheFirst Can you guys check if I broke anything in this PR?

@saecki
Copy link
Contributor Author

saecki commented Sep 26, 2022

Ok so that definitely broke something

@saecki
Copy link
Contributor Author

saecki commented Sep 26, 2022

This feels a little bit inconsistent, but I've found it to actually work quite well, though other suggestions are welcome.
If auto_bounds is active, changing the data_aspect works as before.
If auto_bounds is inactive, changing the data_aspect only changes the y-axis.

Screencast.from.2022-09-26.17.59.43.webm

@saecki saecki marked this pull request as ready for review September 26, 2022 16:09
@emilk
Copy link
Owner

emilk commented Oct 2, 2022

There are now merge conflicts with #2029, but it should be pretty trivial to fix if you merge in master and apply:

index a517db1e..86faad9a 100644
--- a/crates/egui/src/widgets/plot/mod.rs
+++ b/crates/egui/src/widgets/plot/mod.rs
@@ -79,6 +79,13 @@ struct AxisBools {
     y: bool,
 }
 
+impl AxisBools {
+    #[inline]
+    pub fn any(&self) -> bool {
+        self.x || self.y
+    }
+}
+
 impl From<bool> for AxisBools {
     fn from(val: bool) -> Self {
         AxisBools { x: val, y: val }

@saecki
Copy link
Contributor Author

saecki commented Oct 2, 2022

Rebased onto master and applied the patch

@emilk emilk merged commit 981fdb3 into emilk:master Oct 2, 2022
@emilk
Copy link
Owner

emilk commented Oct 2, 2022

thanks!

@saecki saecki deleted the data_aspect branch October 2, 2022 13:14
@Zoxc
Copy link
Contributor

Zoxc commented Oct 2, 2022

Note that the auto_bounds variable is something entirely different after #2029 and it doesn't seem to make sense with the use in this PR.

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.

Plot always zooms out when changing data aspect
3 participants