Make long menus optionally scrollable instead of always expanding horizontally
Needs ReviewPublic

Authored by ngraham on Mar 1 2019, 3:06 PM.

Details

Reviewers
broulik
Group Reviewers
Breeze
Summary

FEATURE: 404942
FIXED-IN: 5.16.0

Test Plan

Config UI:

In a VM, toggle the setting, make the screen size tiny, open Dolphin, and open its Control menu

"Extend Horizontally" setting:

"Scroll" setting:

Diff Detail

Repository
R31 Breeze
Branch
scrollable-humongous-menus (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9037
Build 9055: arc lint + arc unit
ngraham created this revision.Mar 1 2019, 3:06 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 1 2019, 3:06 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Mar 1 2019, 3:06 PM
cfeck added a subscriber: cfeck.Mar 1 2019, 3:23 PM

Please not. I use wide menus extensively, e.g. for bookmarks, and are not keen to wait eons to find an item because of the slow scrolling.

This issue has an origin from : https://github.com/psifidotos/applet-window-appmenu/issues/45

menus dont respect the available screen size

FWIW, there is a way to do it on a per-menu basis using a combination of

QWidget::setStyle and QProxyStyle

FWIW, there is a way to do it on a per-menu basis using a combination of

QWidget::setStyle and QProxyStyle

Not sure that would really help in this specific case since the bug reporter wants his bookmarks menu to be scrollable rather than expanding, while @cfeck wants the same menu to be expanding rather than scrollable!

I suppose (sigh) I could make it configurable...

cfeck added a comment.Mar 1 2019, 3:44 PM

The problem is that adding this property limits to a single column. It should show all columns, and additionally allow the user to scroll. For a menu that only has a few more items than fit in a column this is acceptable, but for the menu in the referenced ticket it is not.

The problem is that adding this property limits to a single column.

Yeah, that's the point. :)

For a menu that only has a few more items than fit in a column this is acceptable, but for the menu in the referenced ticket it is not.

Well the person who filed that ticket clearly didn't find the expandable menu acceptable, or else they wouldn't have filed it! :)

ndavis added a subscriber: ndavis.Mar 1 2019, 4:00 PM

Horizontal menus are fast, but unusable or hard to read if they get too large. Scrollable menus with the arrows that you hover over are annoyingly slow if you don't use a scroll wheel to scroll, but don't block the entire screen if you have a large number of items. In both cases, the best solution from the user's end is to make more folders so that the list doesn't overflow.

cfeck added a comment.Mar 1 2019, 4:06 PM

What he complains about is that items are unreachable. Yes, it should scroll. Either horizontally or vertically. A scrollbar would be perfect so you see how much you don't see. In addition to multiple columns, if there are 1½ or more.

Maybe making it configurable would make most users happy :)

Another unexpected behavior in this issue is that if you are using the Global Menu widget, the emerging menu (that supposedly doesn't fit the screen) shouldn't cover the panel that has the Global Menu widget.

ngraham updated this revision to Diff 52921.Mar 1 2019, 8:48 PM

Make it configurable and don't change the default behavior

ngraham retitled this revision from Make long menus scrollable instead of expanding horizontally to Make long menus optionally scrollable instead of always expanding horizontally.Mar 1 2019, 8:50 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
cfeck added inline comments.Mar 1 2019, 9:02 PM
kstyle/breezestyle.cpp
678
if (cond) {
    return true;
} else {
    return false;
}

return cond ? true : false;

return cond;
ngraham updated this revision to Diff 52924.Mar 1 2019, 9:07 PM
ngraham marked an inline comment as done.

Simplify the conditional

FWIW, there is a way to do it on a per-menu basis using a combination of

QProxyStyle is super heavy as it creates an entire new instance of BreezeStyle. My approach would have been to bodge KStyle to read menu->property("_kde_scroll_menu").toBool() and than use that in the rare instance where we want scrolling (e.g. global menu)