Segmentations module: clicking on slider track should move slider in steps; spinner should increment last digit

Support Feature requests
1

The Segmentations module has several sliders in the Display panel, all showing values from 0.00 to 1.00 (to two decimal places). These are accompanied by number boxes (text fields) with spinners, and tick-boxes (check-boxes).

Current behaviour:

  • tick-box toggles the effective value to either 0.00 or the user’s preference;
  • dragging the slider knob moves in steps of size 0.10;
  • clicking the slider rail jumps to either 0.00 or 1.00;
  • clicking the spinner moves in steps of size 0.10;
  • any desired (valid) number can be typed into the text field.

Desired behaviour:

  • tick-box toggles the effective value to either 0.00 or the user’s preference (no change);
  • dragging the slider knob moves in steps of size 0.05 or 0.01 (medium or fine resolution);
  • clicking the slider rail moves in steps of size 0.10 (coarse resolution);
  • clicking the spinner moves in steps of size 0.05 or 0.01 (medium or fine resolution);
  • any desired (valid) number can be typed into the text field (no change).

The suggested GUI changes are consistent with the behaviour of vertical scrollbars in a wide variety of applications such as in word processing, PDF document reading/editing, and spreadsheet processing. That is, clicking the scrollbar acts like Page up or Page down (large steps), not like Home or End; dragging the scrollbar allows fine control (steps as small, or smaller, than by clicking the arrows at the end of the bar).

The current behaviour of clicking the slider rail is particularly useless, because most of the sliders start with a default value of 1.00, so the current slider rail behaviour in those cases is a redundant duplication of the tick-box.

—DIV

2

Code in question lives here:

^ These would both be accomplished by changing the singleStep property to the desired new value.

^ This would be accomplished by changing the pageStep property to the desired new value.

@DIV Could you issue a PR Pull requests · Slicer/Slicer · GitHub with these changes?

3

I’d rather leave it to the experts :stuck_out_tongue:
Otherwise you’ll have to step me through the process. I clicked on the link, and read some documentation, but for a pull request it seems I must maybe make a new branch or a new fork or make some edits on an existing branch? At the link for the code the pencil editing button shows “You must be on a branch to make or propose changes to this file”.
—DIV

4

Page size is typically 30% in most of the percentage sliders, but I agree that 10% page step would be a bit more useful.

Step size in Powerpoint is set so that you can get from 0% to 100% using the up/down spin sliders in about 6 seconds. This would mean about 5% single step size in Slicer (on my computer).

You can go on and make the change and github will offer to create the fork. So, modifying a single file and creating a pull request is trivial.

However, we would need to the change all the 0-100% sliders across the application the same way (unless there is a specific reason to make the step sizes different). That would mean modifying several .ui files. For this, after github creates the fork and branch, you can go to your fork and modify additional .ui files there. Github will add those changes to the pull request automatically.

You experiment with github and creating pull requests, forking the repository, etc. as you cannot mess up anything in the official Slicer repository.

5

Sounds reasonable, and I think compatible with my suggestions.

Then it sounds like someone (or a small team) with excellent familiarity with the whole project would be better placed to make these changes systematically across all of the relevant files.

I am not completely averse to experimenting and making changes on github, although it would be nice to have some basic pointers — like which branch to work with (nightly? 4.11?), or how to make the edit if the ‘pencil’ button just produces an error. Not much point spending time making changes on files that that nobody else can access, for example, or which can’t be readily merged with the ‘active’ development code.
Of course, there’s also a balance in allocating the time between my ‘day job’ and learning about github & perhaps making some small contributions there.

—DIV

6

All developments and fixes are done in the master branch.

We don’t do anything special in Slicer’s github repository. Suggesting file modification works the same way as in all other repositories.

The You must be on a branch to make or propose changes to this file message means* that you need to choose a branch where you want to modify the file on. So, select the master branch in the version selector near the top-left corner:

image
image895×633 50.2 KB

All of Slicer’s source code is on github, you have full read access the latest master version, which is the same that all developers are actively working on. You also have full write access on your fork (that github creates for you automatically when you edit a file). You can make all changes in your fork and then create a pull request to ask for your proposed changes integrated into the official repository.

We appreciate your feedbacks (it is particularly valuable to get perspective of a new user). Learning the github contribution process can be useful in general and finding percentage sliders in the Slicer GUI takes similar effort for everyone, but if you cannot work on these now it is fine, we understand that you need to prioritize (same for all of us). Just let us know what you plan to do to make sure there is a follow up.

1 Like
7

I just wasn’t familiar with the ‘usual’ way of doing things on GitHub either.
FWIW, perhaps for others on the forum, I also came across these “Contributing to Slicer” guidelines. Somewhat over my head, though.

Re-reading my comment, I realise it may have been unclear. I was not making an assumption or assertion, and I wasn’t referring to the branches that I can see (which must ipso facto be public). I was indicating that hypothetically — for all I know — I might end up creating a ‘private fork’ (if such a thing exists) that nobody else can access.
Anyway, I’ve had a go now, and maybe it will work.

I’ve had a go on the indicated file for the Segmentations module, which was the interface that was bothering me. If nothing else, at least this might give an indication as to whether my Pull Request ended up in the right place.
I haven’t touched any other files/modules, and don’t have that as a priority.

I appreciate your additional help in introducing me to the GitHub process. Like most things, I imagine that the learning curve is steeper at the start.

—DIV

8

Your pull request has been merged. Page/single step of percentage sliders will be updated in tomorrow’s Slicer Preview Release. Thank you for your contribution.

1 Like
9

Could someone with knowledge of github please take a look at

I tried making some basic changes to the slider code in the Model module but I am getting errors such as

Modules/Scripted/SegmentStatistics/SegmentStatistics.py:559: [B020] Found for loop that reassigns the iterable it is iterating with each iterable value.
    for name in {name for name in uniqueHeaderNames if uniqueHeaderNames.count(name)>1}:
        ^

Error: The process '/opt/hostedtoolcache/Python/3.9.12/x64/bin/pre-commit' failed with exit code 1

which may be nothing to do with my changes.

I also tried to amend my own changes, but not sure if that was done correctly.

—DIV

10

It also fails “CommitCheck — Not all of your commit messages are valid.”, but I am unable to see what is the offending part of the message.

11

You can learn a little about CommitCheck from the post below or more directly at Style Guide — 3D Slicer documentation.

1 Like
12

Thanks, James.
I do now recall that a prefix was mentioned in one of the guides, which I omitted.
So I think I have fixed that, although there seems to be no way to get the check to be re-run (except perhaps making more edits to the code).

Also the link on GitHub to “Details” of the CommitCheck check doesn’t work for me.
I click and get taken immediately to https://www.commitcheck.com/401, which says “_ Something went wrong … You may have been logged out._” and provides a button to “Log in with GitHub” …but when I click to do that, I’m taken to https://www.commitcheck.com/users/auth/github, which shows only “Not found. Authentication passthru.”.

Any ideas on the linting error?
At this point I’m guessing that the code hasn’t changed, but the linting check has been modified since the original code was written.

—DIV

13

CommitCheck is checking the prefix of the Commits in the PR and not the PR title as you have recently updated. Your PR currently includes commits titled Harmonising slider step sizes. and Update qMRMLModelDisplayNodeWidget.ui which neither have an appropriate commit prefix. You should be able to find resources online discussing how to squash and amend commits. I suggest you practice in a separate branch first before pushing updates to the branch corresponding to your PR.