Update Play slider #2
Conversation
| def value(self) -> int: | ||
| return self._spinbox.value() | ||
|
|
||
| def setValue(self, value: int): | ||
| self._spinbox.setValue(value) |
There was a problem hiding this comment.
Best to provide an API than accessing private attributes
| self._build() | ||
|
|
||
| def _build(self): | ||
| self._framerate_action = SliderPlayWidgetAction(self, label="FPS:", tooltip="Display speed in frames per second") |
There was a problem hiding this comment.
Let's use CamelCase everywhere in Qt stuff
| @@ -294,13 +329,17 @@ def __init__(self, parent=None): | |||
| self._playButton.setToolTip("Display movie with frames") | |||
| self._playButton.setIcon(icons.getQIcon("camera")) | |||
| self._playButton.setIconSize(iconSize) | |||
| self._playButton.setCheckable(True) | |||
There was a problem hiding this comment.
Use button checkstate rather than changing the icon (or the a real stop action should be used)
| self.mainLayout.addWidget(self._playButton) | ||
|
|
||
| self._playButton.clicked.connect(self._playStopSequence) | ||
| self._playButton.toggled.connect(self._playButtonToggled) |
There was a problem hiding this comment.
Use toggled rather than clicked so it is also called when set programmatically.
| self._menuPlaySlider = _PlayButtonContextMenu(self) | ||
| self._menuPlaySlider.sigFrameRateChanged.connect(self._frameRateChanged) |
There was a problem hiding this comment.
Update the timer interval when changing the frame rate: This enables changing the frame rate while playing
| def spinBoxFrameRate(self) -> qt.QSpinBox: | ||
| """Returns the SpinBox widget for FrameRate display.""" | ||
| return self._menuPlaySlider._framerate_action._spinbox | ||
|
|
||
| def spinBoxInterval(self) -> qt.QSpinBox: | ||
| """Returns the SpinBox widget for interval display.""" | ||
| return self._menuPlaySlider._interval_action._spinbox | ||
|
|
There was a problem hiding this comment.
Even if it is done for other widgets here, it is IMO not a good idea to expose internal widgets through the public API, it's best to provide the API to get/set the corresponding state.
| def setInterval(self, value: int): | ||
| """Set the Interval value for the PlaySlider""" | ||
| self.spinBoxInterval().setValue(value) | ||
| def setPlayImageStep(self, value: int): |
There was a problem hiding this comment.
Proposition to replace Interval which is quite generic (e.g., QTimer.setInterval which set the time interval) with something more specific.
Any better proposition welcomed
| def _playButtonToggled(self, checked: bool): | ||
| """Start/Stop the slider sequence.""" | ||
| if self.__timer.isActive(): | ||
| self._stopTimer() | ||
| else: | ||
| self._startTimer() | ||
|
|
||
| if checked: | ||
| self.__timer.start() | ||
| return | ||
| self.__timer.stop() |
There was a problem hiding this comment.
Rework the QTimer and button to make it simpler: use the check state of the button to handle the QTimer
Some propositions of changes for PR silx-kit#4176