Wrap selection around at the bottom/top of the library's tracks list

[robbert-vdh]

As described in #11088. This makes navigating key-sorted track lists using a controller much more convenient. Though as mentioned in the comment, this does not match the Up/Down cursor key behavior. Should that maybe also cause the tracks list to wrap around? On a keyboard you have Home/End to go to the start and end of the list, but there I'd also expect the cursor keys there to wrap the list around in a similar way. See below.

[robbert-vdh]

Since it made a lot of sense to me both consistency and functionality wise to also use this same behavior for Up/Down cursor key presses, I ended up changing the implementation to instead add the wraparound directly to the cursor movement handler in the libraryWLibraryTableView widget. The added benefit is that this also interacts nicely with the other common selection hotkeys, like Shift+Up/Down to select the previous/next row and Ctrl+Up/Down to focus a row without selecting it (+ Space to actually toggle selection).

Let me know what you think.

[ronso0]

Thank you, this looks good. Overriding moveCursor is smart move IMO. IIRC I didn't consider the move only the cursor. Looks like it would make sense to adjust the Home/End key handler from WTrackTableView::keyPressEvent as well? Atm it always moves the selection, Ctrl has no effect. unfortunately QAbstractItemView::MoveHome / ..MoveEnd also affect the column IIUC so this would require handling the modifiers?

[ronso0]

Btw, I wondered why we actually have WTrackTableView and WLibraryTableView, so I moved moveCursor and moveSelection to WTrackTableView (and it works as before). I didn't find a place where WLibraryTableView is used. Is this just legacy from previous implementations? Any idea @daschuer ?
I think we may simply merge those two classes, no? Then we'd have all methods dealing with cursor and selection in one place.

[robbert-vdh]

Looks like it would make sense to adjust the Home/End key handler from WTrackTableView::keyPressEvent as well? Atm it always moves the selection, Ctrl has no effect. unfortunately QAbstractItemView::MoveHome / ..MoveEnd also affect the column IIUC so this would require handling the modifiers?

Yeah sure! So you're referring to the Shift/Ctrl handling, right? I took a peek at how this works in QAbstractItemView and it doesn't like like there's a helper function for this, so we'd probably need to copy some bits and pieces from that.

[ronso0]

Yes, that's what I mean.
So it would work like this, right?
press Home/End

• (no modifiers): jump to and select first/last row, keep focused column
• with Ctrl: focus first/last row, keep focused column, keep selection, select with Space
• with Shift: focus cell in first/last column in focused row, keep selection, select with Space
  Edit or should Shift+Home just select a range? So this would be Ctrl+Shift?

[robbert-vdh]

I'll take a look at that tomorrow. Want me to tack it onto this PR or should I create a second one?

[ronso0]

I think a separate PR is better because this one can be merged as soon as we did some testing, and agreed to make wrapping around the default.

Actually it depends on where you want to implement Home/End modifiers. If you do it in moveCursor() you may have to rebase this PR since the new will probably get merged quicker, it's a small improvement = doesn't break anything.
You can also modify keyPressEvent first, then --when it's merged-- move it to moveCursor() in this PR here. 🤷

[ronso0]

@mixxxdj/developers Does anyone have objections making Up/Down keys wrap around?

[daschuer]

No Objections.

The code looks good and the cursor behaves natural when using Keyboard.
Thank you very much.

[ronso0]

Btw, I wondered why we actually have WTrackTableView and WLibraryTableView, so I moved moveCursor and moveSelection to WTrackTableView (and it works as before). I didn't find a place where WLibraryTableView is used. Is this just legacy from previous implementations? Any idea @daschuer ? I think we may simply merge those two classes, no? Then we'd have all methods dealing with cursor and selection in one place.

@daschuer Any thoughts on that? Continued in #11100 (comment)

[ronso0]

I'm torn apart regarding this feature:

1. yes, now I don't have to hit Home/End anymore to get to the other end of the list, but I need it to rarely it would make up for 2:
2. wrapround (scroll "overshoot") is still surprising / confusing because there's no clear feedback when I reach the top/bottom

That is due to the default scroll behaviour: the selection needs to reach the top/bottom of the viewport before it'd scroll the view.

Possible fixes:

• re-implement scrolling:
  always show 1-3 lines below/above the selection while scrolling
  might be rather difficult..
  could help with the track 'overview'
  not sure if 3 lines would help when moving fast
• "stop" when hitting the top/bottom:
  when sending Up/Down commands rapidly, require a pause before wrapping around

What do you think?

[robbert-vdh]

Always keeping 1-3 lines of context when scrolling is a good idea regardless. And I do still think wraparound without pauses or anything is a necessary feature. If you're scrolling through the list with the cursor keys then you may as well use a mouse which is faster. But if you're using a controller to navigate you won't have the equivalent of a home/end key to bring you to the start/end of the list. Wraparound is the only logical behavior as I see it.

[ronso0]

I'm using a controller and I still prefer to hit Home when I'm midway in the tracks.

Do you have pointers how to get more context when scrolling?

[robbert-vdh]

I've never used Qt so I'd also need to dig through the APIs first. I doubt something like this would already exist within Qt, but the idea would be that whenever the selection/focus changes the view would be scrolled as necessary to make sure the 2-3 items above and below the focused item are visible. With some extra behavior to reduce this number as the widget's height decreases and fewer items fit (so if only three rows fit it would try to keep one above and one below the selected row visible).

[ronso0]

well, for the time being, this is my personal workaround:
https://github.com/ronso0/mixxx/tree/tracks-wraparound-delay
when pressing Up/Down repeatedly, require a keypress pause of 1 second before wrapping around. 1 second is just a guess, will probably adjust that after testing with my controller.
also, this can be adjusted to work with one timer, but 🤷‍♂️

[ronso0]

and show some lines above/below when moving the cursor 9d39314

I filed #11276