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

Refactor of ROIPropertiesTableWidget #2500

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

samtygier-stfc
Copy link
Collaborator

@samtygier-stfc samtygier-stfc commented Feb 18, 2025

Work on #2378

Also closes #2499 (I think the issue was due to nested tables in the layout making Qt unhappy)

Description

Now that ROIPropertiesTableWidget is split into its own class, it is easier to make some refactoring.

This PR:
Does the layout for ROIPropertiesTableWidget with a .ui file
Moves all access to internal widgets into the class, with high level access functions (e.g. set_roi_limits())
Uses explicit widget names, rather than looping over keys to make any ordering difference bugs impossible

Developer Testing

  • I have verified unit tests pass locally: make check and make test-system
  • I have maunally tested

Acceptance Criteria and Reviewer Testing

Note changes are split into commits for easier reviewing

  • Adding and removing ROIs
  • Resizing ROIs by dragging handles in the image view
  • Resizing ROIs using the spin boxes
  • switching between samples

Documentation and Additional Notes

Does not need release notes as part of bigger work

Will have some screenshot changes

Define layout with a grid layout instead of a table
And move the changing into the class
No where outside class has direct access to spinboxes any more.
Remove dicts of widgets
Use explict access to widgets, so it is impossible for properties to be in the wrong order
Remove unused function
@samtygier-stfc
Copy link
Collaborator Author

This causes some small changes to the layout
image

@coveralls
Copy link

Coverage Status

coverage: 73.101% (+0.5%) from 72.645%
when pulling d85c5cd on test-new-roi-props
into fa2343c on main.

@samtygier-stfc samtygier-stfc marked this pull request as ready for review February 18, 2025 13:44
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.

Switching tabs in spectrum viewer does not work
2 participants