-
Notifications
You must be signed in to change notification settings - Fork 82
silx.gui.plot.PlotWidget: Allow directive zoom with shortcut ALT and SHIFT like h5web #4631
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,9 +52,9 @@ def __init__(self, plot: PlotWidget, parent: qt.QWidget | None = None): | |
| self.__plotRef = weakref.ref(plot) | ||
|
|
||
| self.addSection("Enabled axes") | ||
| self.__xAxisAction = qt.QAction("X axis", parent=self) | ||
| self.__yAxisAction = qt.QAction("Y left axis", parent=self) | ||
| self.__y2AxisAction = qt.QAction("Y right axis", parent=self) | ||
| self.__xAxisAction = qt.QAction("X axis (alt)", parent=self) | ||
| self.__yAxisAction = qt.QAction("Y left axis (shift)", parent=self) | ||
| self.__y2AxisAction = qt.QAction("Y right axis (shift)", parent=self) | ||
|
Comment on lines
+55
to
+57
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me here there is clear need to clarify the policy we want to have. So from my understanding either this is added at the silx view application level and/or this should be optional at the lib side.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think if we add this shortcut it can benefit to library too ?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agreed. It should not enabled by default in the lib but be in opt-in. And
Yes, it would be good to still have in the library so that other consumer apps can enable it. But not enabled by default.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So far, plot interactions are mostly wired in I agree silx should leave it up to the application to choose it's keyboard short-cuts (unless they are standard ones). However here it is about modifier keys for the mouse wheel which is a behavior I don't think one cannot infer with from the API.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i m not sure i understand the final decision @t20100 . So i can keep thit like that and let it in silx core ?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless someone disagree, I would leave it as it is in silx core. |
||
|
|
||
| for action in (self.__xAxisAction, self.__yAxisAction, self.__y2AxisAction): | ||
| action.setCheckable(True) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when alt is pressed in opengl, event.angleDelta().y() has a value and event.angleDelta().x() is 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect x to always be
0except for fancy mouses no ? According to the docAnd even if x is not zero I am not sure we want to take it into account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry my comment was wrong when alt is pressend in open gl x value is set else y
that's why. DO you want me to be more specific like really use x when alt is pressed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior is hard-coded in qt xcb plugin: https://github.com/qt/qtbase/blob/120883a028a59864dc7691dd1efa318bd602755d/src/plugins/platforms/xcb/qxcbwindow.cpp#L1955-L1956
Rather than summing, it's also possible to use
x()only wheny() == 0Since this is not documented in qt doc, it's worth a comment to explain!