search

lxqt / lxqt-panel

The LXQt desktop panel
https://lxqt-project.org
GNU Lesser General Public License v2.1
179 stars 135 forks source link

FancyMenu: share code with MainMenu #2005

Closed gfgit closed 5 months ago

gfgit commented 5 months ago

This PR aims at sharing common code between the 2 menu implementations. The common code is not much but sharing makes it possible to keep behavior consistent with less effort I think.

TODO:

  1. where to put common files?
  2. Is the shortcut hack still needed? Can it at least be documented better, maybe linking Qt BUGs etc?
  3. Some includes are not needed anymore so I removed them and reordered based on their function. Still better job could be done here.
tsujan commented 5 months ago

Since Fancy Menu is really not a QMenu and could go its own direction, I think sharing codes might be an obstacle to development later..

tsujan commented 5 months ago

@gfgit BTW, and unrelated to this:

Although the changes I made weren't complex, I wanted to request your review on a few occasions but didn't know if you were accessible. Will you be accessible for reviews in future?

gfgit commented 5 months ago

@gfgit BTW, and unrelated to this:

Although the changes I made weren't complex, I wanted to request your review on a few occasions but didn't know if you were accessible. Will you be accessible for reviews in future?

Right now I may not answer immediately, but in general yes.

gfgit commented 5 months ago

Since Fancy Menu is really not a QMenu and could go its own direction, I think sharing codes might be an obstacle to development later..

Well if shared code becomes less useful it can always be reverted back. I think the shortcut hack is quite bad code so it should stay around only until better approach is found. Making it shared means when it will be removed, both menus will benefit from this, otherwise one might forget and update only one of them.

This is similar to a recent change you made about wayland and explicitly setting Qt window flags in constructor. While that's a single line so cannot be shared, I probably would never read it because I don't read full repository code when I'm just working on a single plugin...

tsujan commented 5 months ago

Well if shared code becomes less useful it can always be reverted back.

That will be a part of the obstacle I mentioned ;) Obstacles could be removed, of course, but why introducing them in the first place?

I think the shortcut hack is quite bad code

Not only that, but also having the same shortcut for Main Menu and Fancy Menu isn't good. Anyway, we'll need another method under Wayland everywhere.

This is similar to a recent change you made about wayland...

It's rather a workaround that might not be needed in future (actually, it wasn't needed here; I added it just for the sake of certainty). Qt's Wayland support has changed a lot in Qt6.

That being said, caring for Wayland doesn't necessitate code sharing. As important examples, some menus and tooltips need a parent widget under Wayland, DND codes should be written more carefully, the cursor position shouldn't be taken for granted, etc. The coder needs to get familiar with these things.

Right now I may not answer immediately, but in general yes.

An immediate answer isn't a requirement. Thanks!