Ticket #3554 (closed defect: fixed)
Listbox shouldn't wraparound on mouse scroll
Reported by: | egmont | Owned by: | andrew_b |
---|---|---|---|
Priority: | minor | Milestone: | 4.8.16 |
Component: | mc-core | Version: | master |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Branch state: | merged | Votes for changeset: | committed-master |
Description
Open a listbox such as Alt-h. Use mouse or touchpad scroll. Notice it wraps around.
Wrapping around may be okay for keyboard up/down (although I'm not even sure about it, as let's say the main panels don't wrap).
For touchpad/mouse it's definitey undesired, since you'd expect a single fast/long scroll motion to end up at the first/last item, not go spinning around and around.
Attachments
Change History
comment:2 Changed 9 years ago by mooffie
Here's a minor code cleanup patch that makes it trivial to fix the wraparound issue.
But this patch would be nice to commit regardless of what we decide to do, if at all, with the wraparound issue. Maybe I should have posted it in a separate ticket?
The patch replaces every:
for (...) listbox_back/fwd (l);
with:
listbox_back/fwd_n (l, ...);
This makes the code easier to decipher (...and it leaves listbox_fwd/back() to the exclusive use of the up/down keyboard or mouse keys, which makes fixing the wraparound issue trivial).
comment:3 follow-up: ↓ 4 Changed 9 years ago by andrew_b
- Owner set to andrew_b
- Status changed from new to accepted
- Branch state changed from no branch to on review
- Milestone changed from Future Releases to 4.8.16
Branch: 3554_listbox_wrap
Initial changeset:99dd99172f4c61ff36450a56cb08634bbd24bad4
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 9 years ago by mooffie
Replying to andrew_b:
This works for me. Excellent.
BTW, the following...
if ((guint) l->pos + 1 < g_queue_get_length (l->list))
...can now be changed to:
if (l->pos < LISTBOX_LAST (l))
comment:7 Changed 9 years ago by andrew_b
The new hidden option 'listbox_wraparound_mouse_scroll' was added to enable/disable listbox wraparound on mouse scroll. Enabled by default to keep current behaviour.
Does anybody vote for this ticket? The last comment was 4 weeks (a month) ago.
comment:8 follow-up: ↓ 9 Changed 9 years ago by ossi
and yet another useless micro option ...
at least switch the default to the new behavior. if somebody looks for the old one, they'll easily find where it has gone.
comment:9 in reply to: ↑ 8 Changed 9 years ago by andrew_b
Replying to ossi:
and yet another useless micro option ...
at least switch the default to the new behavior. if somebody looks for the old one, they'll easily find where it has gone.
There is no problem to remove last commit. I'm not insist on this option.
comment:10 Changed 9 years ago by andrew_b
- Votes for changeset set to andrew_b
- Branch state changed from on review to approved
comment:11 Changed 9 years ago by andrew_b
- Status changed from accepted to testing
- Votes for changeset changed from andrew_b to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Merged to master: [1cf18abd62bcba6d636ee955eb29fc9b26548756].
git log --pretty=oneline a5cd009..1cf18ab
without micro option.
wrapping is also inconsistent with other ui elements (panels don't wrap, and editors and viewers naturally don't, either.
wrapping can be an optimization in some cases, but it's unintuitive enough to be not useful imo (i always hit 'end' first anyway). i'm all for removing it.