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 autoscale button to disable automatic y-axis scaling #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davrsky
Copy link

@davrsky davrsky commented Dec 5, 2017

Sometimes I don't want y-axis to automatically scale, adds a checkbox to enable/disable it

@@ -154,6 +155,10 @@ def switch_data_plot_widget(self, data_plot):
self.data_plot = data_plot
self.data_plot_layout.addWidget(self.data_plot)
self.data_plot.autoscroll(self.autoscroll_checkbox.isChecked())
if self.autoscale_checkbox.isChecked():
self.data_plot.set_autoscale(y=DataPlot.SCALE_EXTEND|DataPlot.SCALE_VISIBLE)
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value SCALE_EXTEND|SCALE_VISIBLE is currently being defined in the plot.py file. If the patch wants to make that toggle within the widget then the default value from the plot file should be removed and instead a function on the widget should be called to achieve the same behavior without duplicating these values.

Given there are two topics:
/foo
/foo/bar

And the user requests to graph
/foo/bar/baz

Previously, it would randomly pick a topic (I think in practice it would
always pick /foo, but not sure). Now I make it pick the longest matching
topic, which is /foo/bar.
@alex-makarov
Copy link

Merging this would boost the usability of rqt_plot a lot. Please merge..

@lucbettaieb
Copy link

Please merge this! rqt_plot gets into an absolutely unusable state very easily...

@lucbettaieb
Copy link

@dirk-thomas <3

@dirk-thomas
Copy link
Contributor

@alex-makarov @lucbettaieb The pending comment stills needs to be resolved. Also there is a second completely unrelated commit on this PR which should be separate into its own PR. If you would like to see either of the two changes merged please consider to open PRs with the comments addressed.

@lucbettaieb
Copy link

@dirk-thomas My bad! I assumed the second commit contained the fixups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants