Fancy Menu: New main menu similar to XFCE Whiskermenu

gfgit commented 11 months ago

Hello, as promised this is a first working draft of "Fancy Menu"

DISCLAIMER: The name is the first thing that came to my mind, git history is messy, will be cleaned up before final version.

TODOs:

[x] Clean up Git history
[x] Clean up settings window
[ ] Share common code with existing Main Menu plugin
[ ] Possibly rename Main Menu plugin to disambiguate
[x] Is QTBUG-52021 specific to QMenu or should I keep the workaround?
[ ] Do you like current theme or should I make a new QStyle subclass?

Related issues: #1870 #1196

Possibly related PRs: #1698 #1771

stefonarch commented 11 months ago

Nice work! Is "adding to favorites" not implemented yet or did I miss something?

gfgit commented 11 months ago

@stefonarch I've not yet implemented it because I'm still deciding where to store the info. Should it be in the same panel.conf file? And should there be a maximum number of items which can be set as favorites?

stefonarch commented 11 months ago

panel.conf should be fine. I don't think a max number is needed, but a sort option would be nice. Atm some of the categories are truncated - a bigger size e/o autoresize to content would help here.

gfgit commented 11 months ago

I've added "add to favorites" but it's not working yet

gfgit commented 11 months ago

Now favorites are stored inside panel.conf. It works well, crash is fixed. I will squash some commits

gfgit commented 11 months ago

I've noticed some parts of lxqt-panel use QIcon::fromTheme() while others use XdgIcon::fromTheme(). I've ported my code to XdgIcon, is it correct?

Examples of code using QIcon::fromTheme():

plugin-backlight/backlight.cpp:36
plugin-statusnotifier/statusnotifierbutton.cpp:60

tsujan commented 11 months ago

Happy to see that you're working on this.

I know that this is a WIP, but please remove translations and also changes not related to Fancy Menu (if any) from the final PR. Translations will be handled by @stefonarch later.

The name is the first thing that came to my mind

It sounds good to me.

tsujan commented 11 months ago

I've noticed some parts of lxqt-panel use QIcon::fromTheme() while others use XdgIcon::fromTheme()

It makes no difference here. You could use the Qt method.

stefonarch commented 11 months ago

Nice work really!

Some things I noticed:

the "Add to favorites" item could go on top
Most settings inherited under "Search" from main menu are irrelevant and could be removed
"Clear search upon show menu" does only clear the textedit - search results are still listed when reopening
Selected categories remain selected at reopening the menu, it should default always to "Favorites" view (should resolve also 4. )
Bigger size of the right panel (categories are elided)?

Very nice that also search results can be added to favorites!

screen_area_dom_18:58:58_ screen_area_dom_18:59:38_

gfgit commented 11 months ago

the "Add to favorites" item could go on top

On top of the context menu there are app-specific actions defined in desktop file (es. "Open Anonymous Window" for Firefox). I think these are used more often than "Add to Favorites". Ideally you change favorites only once in a while. So I would say it should go as last, maybe with a separator to be more visible. Let me know.

For the other issues I've already noticed them, will fix next week.

About translations I can provide the Italian one, the other files are just copy renamed from main-menu plugin

stefonarch commented 10 months ago

So I would say it should go as last, maybe with a separator to be more visible. Let me know.

I meant at the top of the "copy" and "add to desktop" items, but a separator is fine too.

tsujan commented 10 months ago

I'll try it when the PR is ready, but @stefonarch's screenshot shows that it should be a very nice plugin.

IMHO, there's no need to try to consider every minute detail in the first PR; a clean code that works is enough. Details, issues and needed features might surface only when several devs and git users start to use it — as is the case with any code — and then, everyone will be able to report and/or work on some of them.

gfgit commented 10 months ago

Rebased on current master and fixed search related issues.

What remains to do:

Share some small parts with Main Menu (shortcut hack, context menu)
Make buttons command configurable
Right click on panel item to change plugin configuration shows empty name. Don't know why.
Find a way to calculate minimum size to not elide category names

gfgit commented 10 months ago

Also when dragging an app to panel quicklaunch, the menu popup is not closed. I don't think it's a bad behavior though

stefonarch commented 10 months ago

Also when dragging an app to panel quicklaunch, the menu popup is not closed. I don't think it's a bad behavior though

It's the same with the mainmenu. Didn't recompile yet but while looking at theming I realized tooltips being weird. Mainmenu shows just "GenericName" from .desktop files (here: "Browser Web")

screen_area_lun_19:07:34_

EDIT: oh, probably you mean that with "Share some small parts with Main Menu (shortcut hack, context menu)"

Make buttons command configurable

What do you mean with this? The buttons displayed at top?

gfgit commented 10 months ago

Tooltips in previous version had keywords appended to test search accuracy. It needs imprpvment, maybe share cose with lxqt-runner so search results are similar.

The context menu code can be shared with main menu. It's a few lines but future improvments to common code would benefit both plugins. Same goes to hackish shourtcut sequence code in plugin event filter. I wonder if we can rempve it entirely and use some standard Qt code.

I think main menu closes itself after drag drop to quicklaunch. But in order to reproduce that we loose standat Qt item view drag and drop code. Meaning we should subclass QListView and do our mouse tracking. If we choose to subclass then also this feature can be shared with Main Menu

gfgit commented 10 months ago

What do you mean with this? The buttons displayed at top?

Yes. We could add also user name and icon (.face file) and some "Switch User" / "Logout" fast button. Each button triggers a commans, I'd like to make these commands configurable so in mobile environment I can launch custom leave dialog with touch support

stefonarch commented 10 months ago

I guess the empty name issue come from an accidental removing of the "translations" folder, tried to readd the translations and after removing and readding the plugin the italian name is shown again here, also the translations of the settings. Btw the *.yaml files should be removed except for the template which needs another wording now.

For the buttons I had the same idea, but I thought that those are just .desktop files, but configuring icons and commands sounds nice.

Readding the plugin no icon is shown, probably the first item in the settings needs to be checked by default
Adding a name it will show "Start" first with empty textedit (same in mainmenu I noticed)
here in both menus doesn't work "custom font size". Has to be a bug never filed, as I see it also in my debian WM
Here dragging from the main menu to quicklaunch menu stays open.

isf63 commented 10 months ago

I compiled this and it's completely usable, albeit for a small amount of bugs. Really the only things I see to adjust are: -- Because there already is Favorites, custom menus, and lxqt-panel's Quick Launch, I don't think the buttons at the top should get overly complicated. -- The left and right displays of menu items would profit from separators.

gfgit commented 10 months ago

fancymenu.desktop.yaml ~~file alone is not enough to display proper name~~

EDIT: I had to clean rebuild and now it generates Name and Comment fields correctly inside .desktop file

gfgit commented 10 months ago

The left and right displays of menu items would profit from separators.

Hi, what do you mean by this? Separator between category items?

isf63 commented 10 months ago

what do you mean by this? Separator between category items?

Main Menu has separators in categories and their sub-menus. It's defined as "\<Separator/>" in menu files

gfgit commented 10 months ago

Also about buttons. Whisker menu has them on bottom, below item views. It seems slightly better. Buttons on top are too near the search line edit in my opinion.

stefonarch commented 10 months ago

Also about buttons. Whisker menu has them on bottom, below item views. It seems slightly better. Buttons on top are too near the search line edit in my opinion.

In the wing menu position was configurable, personally I'm fine with both. There are the same icons for "Shutdown" and "Leave", looking at the translations I assume it's "Shutdown" at the moment, IMHO the "Leave" menu should be default.

Translations can be reused from mainmenu plugin so there are only few new items to translate. I've them ready but I see that "Favorites" is not translatable atm.

For the menu file: I see that the default lxqt-applications.menu results in a catchall named "Preferences" while "compact" menu shows both "Preferences" and "LQXt Settings", therefor probably better using this by default.

stefonarch commented 10 months ago

Possibly rename Main Menu plugin to disambiguate

I'm afraid that is not backwards compatible as it will remove the menu when users update.

gfgit commented 10 months ago

I'm afraid that is not backwards compatible as it will remove the menu when users update.

Only if you change the plugin name. But if you just change name in desktop file I think it should be fine. Anyway it'not really important, users can easily distinguish the two types of menus

gfgit commented 10 months ago

@stefonarch Now you should be able to translate everything.

I found a bug in theming: In LXQt Appearance settings I have "Fusion" Qt Style set. If I switch to "Windows" everything becomes more squared. If I switch back to fusion style, Appearance settings windows sets it correctly but FancyMenu popup stays squared.

GitHub doesn't let me attach screenshot for some reasons...

tsujan commented 10 months ago

I found a bug in theming:

On-the-fly applying of themes can never be 100% OK, although its problems are mostly about translucency (which most style engines lack — hence not noticed by many users). App restart is recommended.

stefonarch commented 10 months ago

Now you should be able to translate everything.

Nice work, perfect! I like much the possibility to choose the positions.

screen_area_gio_19:27:41_

Does restart panel (from session settings > modules) fix the appearance? I'm using kvantum+"system" theme, the styling is already ok with it, a bit narrow the items though. As many users are using one of the themes, those have to be updated, I did a test for the first, Ambiance. https://paste.debian.net/1303809/

Anyway, from my point of view this is quite ready to go, maybe "popup" isn't clear, "Buttons"? And the auto resize of the categories, or just a bigger overall size.

Ooops, typo in translation ;)

gfgit commented 10 months ago

On-the-fly applying of themes can never be 100% OK, although its problems are mostly about translucency (which most style engines lack — hence not noticed by many users). App restart is recommended.

I understand but it's even more weird that Fusion->Windows works instantly while Windows->Fusion does not change anything. Anyway restarting panel fixes this minor issue

stefonarch commented 10 months ago

PS: I forgot "Shutdown" button text should be changed to "Leave" as in fact it's the choice.

tsujan commented 10 months ago

it's even more weird that Fusion->Windows works instantly while Windows->Fusion does no...

Some styles may not do a good job in their QStyle::unpolish(), and so, they may "pollute" the next applied style, although even if they do it correctly, there's no guarantee for a clean style change in all cases.

gfgit commented 10 months ago

I've added a new file lxqtfancymenutypes.h. Should it have copyright notice on top? If yes which parts should I copy on it and from where

tsujan commented 10 months ago

... which parts should I copy on it and from where

I think plugin-statusnotifier is a good example; take a look at https://github.com/lxqt/lxqt-panel/blob/master/plugin-statusnotifier/dbustypes.cpp

gfgit commented 10 months ago

Unfortunately I can confirm QTBUG-52021 affecting QLineEdit is present.

Logging QStyle::pixelMetric() I get following output in loop:

PIXEL METRIC: QStyle::PM_DefaultFrameWidth QLineEdit(0x5618ee1d8190)
PIXEL METRIC: QStyle::PM_SmallIconSize QLineEdit(0x5618ee1d8190)
PIXEL METRIC: QStyle::PM_SmallIconSize QLineEdit(0x5618ee1d8190)
PIXEL METRIC: QStyle::PM_DefaultFrameWidth QLineEdit(0x5618ee1d8190)
PIXEL METRIC: QStyle::PM_SmallIconSize QLineEdit(0x5618ee1d8190)
PIXEL METRIC: QStyle::PM_SmallIconSize QLineEdit(0x5618ee1d8190)

Built with Qt 5.15.10 provided by Ubuntu repositories

gfgit commented 10 months ago

@isf63 I've added separator support, could you test if it works for you? Can separators be in-between top level categories too?

For the menu file: I see that the default lxqt-applications.menu results in a catchall named "Preferences" while "compact" menu shows both "Preferences" and "LQXt Settings", therefor probably better using this by default.

The default menu file is returned from XdgMenu::getMenuFileName() which does not consider "compact" version. Also the "Leave" sub-menu is somewhat redundant since there is dedicated "Leave" button.

stefonarch commented 10 months ago

Well, also the "Preferences" is redundant, but removing those will result in those items not being found by the search (not so bad with leave items but not recommendable for all users with preferences/lxqt-settings). I've used a wing-menu without "Preferences".

We could consider to make this menu the default menu and add a basic configuration as we do for the main menu to panel.conf. But this will affect only fresh users/installs.

The separator here shows up on bottom both with "compact" and "lxqt-applications.menu":

screen_area_ven_19:11:20_

isf63 commented 10 months ago

@gfgit, The separators work for me. @stefonarch, they're more visible on the Fusion style. That bottom bar is a scrollbar - on Fusion I get one for the top-level menu, and on a few submenus with wide entries. screenshot

tsujan commented 10 months ago

The separators work for me

It also works for @stefonarch, as far as I can see in his last screenshot. Most Kvantum themes use a configurable vertical space instead of a menu separator (because menu separators are ugly to me, and those themes are made by me).

Anyway, I think details like whether a menu separator should be used here or whether the horizontal scrollbar should be avoided can be dealt later, after the PR is merged. No dev can predict everything single-handedly.

I'll test and check the code soon and might start a review if needed.

stefonarch commented 10 months ago

Oops, that is indeed a scrollbar ;) I expected to see the separators where there are in the menu files, but anyway this is ok.

tsujan commented 10 months ago

Here Fancy Menu makes the panel crash on two occasions: (1) when it's removed from the panel; and (2) when the panel's process is stopped. No crash happens without adding Fancy Menu if the panel's process is stopped.

I didn't check the code, but the backtrace wasn't informative in either case. For (1):

#0  0x000000000065006d in  ()
#1  0x00007f14caca02ad in QMetaObject::cast(QObject const*) const () at /usr/lib/libQt5Core.so.5
#2  0x00007f14cba29096 in  () at /usr/lib/libQt5Widgets.so.5
#3  0x00007f14cb9a9102 in QWidget::isActiveWindow() const () at /usr/lib/libQt5Widgets.so.5
#4  0x00007f14cb9a20bf in QWidget::palette() const () at /usr/lib/libQt5Widgets.so.5
#5  0x00007f14cb9b4f4d in QWidget::initPainter(QPainter*) const () at /usr/lib/libQt5Widgets.so.5
#6  0x00007f14cb3c6582 in  () at /usr/lib/libQt5Gui.so.5
#7  0x00007f14cb3d2f17 in QPainter::begin(QPaintDevice*) () at /usr/lib/libQt5Gui.so.5
#8  0x00007f14cba53fdd in QFrame::paintEvent(QPaintEvent*) () at /usr/lib/libQt5Widgets.so.5
#9  0x00007f14cb9af040 in QWidget::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#10 0x00007f14cba5da83 in QFrame::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#11 0x00007f14cb9788ff in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
#12 0x00007f14cac9bef8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
#13 0x00007f14cb9a2f7b in QWidgetPrivate::sendPaintEvent(QRegion const&) () at /usr/lib/libQt5Widgets.so.5
#14 0x00007f14cb9a437d in QWidgetPrivate::drawWidget(QPaintDevice*, QRegion const&, QPoint const&, QFlags<QWidgetPrivate::DrawWidgetFlag>, QPainter*, QWidgetRepaintManager*) () at /usr/lib/libQt5Widgets.so.5
#15 0x00007f14cb9846d9 in  () at /usr/lib/libQt5Widgets.so.5
#16 0x00007f14cb9af0d1 in QWidget::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#17 0x00007f14cba5da83 in QFrame::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#18 0x00007f14cb9788ff in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
#19 0x00007f14cac9bef8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
#20 0x00007f14caca0e5b in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
    at /usr/lib/libQt5Core.so.5
#21 0x00007f14cace6ec8 in  () at /usr/lib/libQt5Core.so.5
#22 0x00007f14cb6fbf69 in  () at /usr/lib/libglib-2.0.so.0
#23 0x00007f14cb75a367 in  () at /usr/lib/libglib-2.0.so.0
#24 0x00007f14cb6fa162 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#25 0x00007f14cacead0c in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /usr/lib/libQt5Core.so.5
#26 0x00007f14cac9ac04 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
#27 0x00007f14cac9c0a3 in QCoreApplication::exec() () at /usr/lib/libQt5Core.so.5
#28 0x00005555cea1d14a in  ()
#29 0x00007f14ca445cd0 in  () at /usr/lib/libc.so.6
#30 0x00007f14ca445d8a in __libc_start_main () at /usr/lib/libc.so.6
#31 0x00005555cea1d9f5 in _start ()

For (2), it was as short as this:

#0  0x0000000000000018 in  ()
#1  0x00007fdf91b7360f in QApplication::~QApplication() () at /usr/lib/libQt5Widgets.so.5
#2  0x000055ddb0132155 in  ()
#3  0x00007fdf920f0cd0 in  () at /usr/lib/libc.so.6
#4  0x00007fdf920f0d8a in __libc_start_main () at /usr/lib/libc.so.6
#5  0x000055ddb01329f5 in _start ()

tsujan commented 10 months ago

Apart from the above change, I think the plugin should have either an icon (preferable) or a text when the user adds it to the panel (currently it's an empty space on the panel by default).

tsujan commented 10 months ago

Also, the categories panel is on the right by default when the user adds the widget to the panel, while "Left" is chosen in the config dialog.

gfgit commented 10 months ago

Also, the categories panel is on the right by default when the user adds the widget to the panel, while "Left" is chosen in the config dialog.

I don't understand how MainMenu plugin sets the icon. In my panel.conf it does not have neither ownIcon nor icon fields and still show "White helix" icon

EDIT: After some digging I've found the icon comes from lxqt-themes repositories which has may Qt StyleSheets. To mimic MainMenu behavior each of these styles should be edited

gfgit commented 10 months ago

I've found another issue which affects also MainMenu: When "Custom Font Size" is checked for first time, the line edit shows by default "12 pt" but the customFontSize setting is not added to panel.conf. So it tries to set zero point size font which triggers Qt warning and results in no-op

stefonarch commented 10 months ago

When "Custom Font Size" is checked for first time, the line edit shows by default "12 pt"...

It shows font size from lxqt-config-appearance, here it's 13px.

stefonarch commented 10 months ago

To mimic MainMenu behavior each of these styles should be edited

This has to be done for sure, qproperty-icon: url(mainmenu.svg); is it for ambiance theme. I see still in ambiance theme that the fixed size creates a vertical scroll bar:

screen_area_dom_21:12:07_

gfgit commented 10 months ago

Thank you for the kind words! Squashed, hope you don't mind the reference to XFCE

tsujan commented 10 months ago

You're welcome.

hope you don't mind the reference to XFCE

Not at all.

Let's give it to git users :)

stefonarch commented 10 months ago

A draft for themes is here: https://github.com/lxqt/lxqt-themes/pull/105

lxqt / lxqt-panel

Fancy Menu: New main menu similar to XFCE Whiskermenu #1975