Skip to content

Use buttons in toolbar #1797

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.

Sign up for GitHub

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

Merged
psavery merged 4 commits into from
Apr 10, 2025
Merged

Use buttons in toolbar #1797

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec6bceb
Move from spinbox to buttons
bnmajor Mar 25, 2025
f5b1c24
Rename change_frame -> shift_frame
bnmajor Apr 10, 2025
bb7da0d
Improve frame value input
bnmajor Apr 10, 2025
d8d5797
Base frame input size on max frames
bnmajor Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions hexrdgui/image_series_toolbar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from pathlib import Path
from PySide6.QtCore import QCoreApplication, Qt
from PySide6.QtWidgets import QGridLayout, QLabel, QSlider, QSpinBox, QWidget
from PySide6.QtWidgets import (
QGridLayout, QLabel, QLineEdit, QPushButton, QSlider, QWidget
)
from PySide6.QtGui import QFontMetrics, QPixmap
from hexrdgui import resource_loader

import hexrdgui.resources.icons
from hexrdgui.hexrd_config import HexrdConfig
from hexrdgui.utils import block_signals


class ImageSeriesInfoToolbar(QWidget):
Expand Down Expand Up @@ -62,6 +65,8 @@ def __init__(self, ims, parent=None):
self.ims = ims
self.slider = None
self.frame = None
self.back_button = None
self.forward_button = None
self.layout = None
self.widget = None

Expand All @@ -74,14 +79,19 @@ def __init__(self, ims, parent=None):

def setup_connections(self):
self.slider.valueChanged.connect(self.val_changed)
self.slider.valueChanged.connect(self.frame.setValue)
self.frame.valueChanged.connect(
self.slider.setSliderPosition)
self.slider.valueChanged.connect(lambda i: self.frame.setText(str(i)))
self.frame.textChanged.connect(
lambda i: self.slider.setSliderPosition(int(i)))
Copy link
Collaborator

@psavery psavery Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
self.frame.textChanged.connect(
lambda i: self.slider.setSliderPosition(int(i)))
self.frame.editingFinished.connect(self.on_frame_edited)

I think we should only trigger an update after the user has finished editing. Otherwise, if they type in 11, for example, it triggers an update with every keystroke (so first loads frame 1 and then loads frame 11).

But editingFinished also does not supply the new text as an argument, so you'd also have to check it with self.frame.text().

Also, we need to do some validation for the text that the user enters. I can type in invalid text like f and it produces an exception. Instead, we should do something like this:

    try:
        val = int(text)
    except ValueError:
        # Not a valid integer. Restore the previous value.
        val = prev_val

    # Clip the value to the min/max range
    val = max(min_val, val)
    val = min(max_val, val)

Notice also we should clip the value to the min/max range. If the user sets it to something outside of the range, force it to go back to be within the range.

self.back_button.clicked.connect(lambda: self.change_frame(-1))
self.forward_button.clicked.connect(lambda: self.change_frame(1))

def create_widget(self):
self.slider = QSlider(Qt.Horizontal, self.parent())
self.frame = QSpinBox(self.parent())
self.frame.setKeyboardTracking(False)
self.frame = QLineEdit(self.parent())
self.back_button = QPushButton('<<')
self.back_button.setFixedSize(35, 22)
self.forward_button = QPushButton('>>')
self.forward_button.setFixedSize(35, 22)

self.widget = QWidget(self.parent())
self.omega_label = QLabel(self.parent())
Expand All @@ -95,10 +105,15 @@ def create_widget(self):
example_label_text = omega_label_text(359.999, 359.999)
text_width = metrics.boundingRect(example_label_text).width()
self.omega_label.setFixedWidth(text_width)
frame_text_width = metrics.boundingRect('9999').width()
self.frame.setFixedWidth(frame_text_width)
self.frame.setAlignment(Qt.AlignCenter)

self.layout = QGridLayout(self.widget)
self.layout.addWidget(self.slider, 0, 0, 1, 9)
self.layout.addWidget(self.frame, 0, 9, 1, 1)
self.layout.addWidget(self.slider, 0, 0, 1, 7)
self.layout.addWidget(self.back_button, 0, 7, 1, 1)
self.layout.addWidget(self.frame, 0, 8, 1, 1)
self.layout.addWidget(self.forward_button, 0, 9, 1, 1)
self.layout.addWidget(self.omega_label, 0, 10, 1, 1)

self.widget.setLayout(self.layout)
Expand All @@ -119,11 +134,11 @@ def set_range(self, current_tab=False):
self.slider.setMinimumWidth(self.parent().width()//2)
if not size == self.slider.maximum():
self.slider.setMaximum(size)
self.frame.setMaximum(size)
self.frame.setToolTip(f'Max: {size}')
self.slider.setToolTip(f'Max: {size}')
self.slider.setValue(0)
self.frame.setValue(self.slider.value())
self.frame.setText(str(self.slider.value()))
self.back_button.setEnabled(False)
else:
self.show = False
self.widget.setVisible(self.show)
Expand All @@ -148,6 +163,7 @@ def setEnabled(self, b):

def val_changed(self, pos):
self.parent().change_ims_image(pos)
self.update_back_forward_buttons(pos)
self.update_omega_label_text()

def update_omega_label_text(self):
Expand All @@ -161,6 +177,17 @@ def update_omega_label_text(self):

self.omega_label.setText(omega_label_text(*ome_range))

def update_back_forward_buttons(self, val):
self.back_button.setEnabled(self.slider.minimum() != val)
self.forward_button.setEnabled(self.slider.maximum() != val)

def change_frame(self, value):
Copy link
Collaborator

@psavery psavery Apr 9, 2025

Choose a reason for hiding this comment

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

Suggested change
def change_frame(self, value):
def shift_frame(self, value):

Can you rename this so it's clear that we are shifting by the value, rather than changing to the value?

with block_signals(self.frame, self.slider):
new_frame = int(self.frame.text()) + value
self.frame.setText(str(new_frame))
self.slider.setSliderPosition(new_frame)
self.val_changed(new_frame)


def omega_label_text(ome_min, ome_max):
# We will display 6 digits at most, because omegas go up to 360
Expand Down
Loading