-
Notifications
You must be signed in to change notification settings - Fork 735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Resizing Applet PopupMenus will go behind panel if panel is set to auto hide #11770
Conversation
Not sure whether there is any difference in regard to this issue between Mint 19.2 and Mint 21.x but at least here in Mint 19.2 it seems to be some delay in either reporting the real value of Below there are two screenshots. I used a previously modified version of the 'Download and upload speed' applet which adds a blank space to the tooltip/menu in order to circumvent the bug. After applying the fix and restarting Cinnamon all was fine. After a small amount of time the menu position again got miscalculated (first screenshot). I kept the pointer over the applet icon and after an amount of time (less than the clock in the panel shows as difference) the menu jumped by itself into its correct position (second screenshot). I can only speculate that either or both of the values involved in the calculation are being polled on a timer, maybe in order to avoid some heavy calculations or whatever. Bottom line is the bug will still show apparently randomly if the fix will be left as is. I also wonder who and why has forbidden me to reply to the original issue where this bug has been reported: |
This PR has a side effect: In cinnamenu applet, if I open the applet with the super key when panel auto hide is on, the applet is positioned higher up as if the panel is showing even though it isn't. This also happens in the menu applet if it's opened with the super key and "force panel to be visible when opening the menu" option is off in the menu applet config. I can easily fix this in cinnamenu though by always using peekPanel(). In fact the only reason I removed peekPanel() was because of bug #10970 |
It feels like the default behaviour should be that if a popupmenu is visible for a panel the panel should always show. It probably needs some work on the stack variable so panels can use that. This way we could depend on it properly and eliminate other problems too. |
@Gr3q >It feels like the default behaviour should be that if a popupmenu is visible for a panel the panel should always show. I agree. There seems no useful purpose in having an applet open and not have the panel show at the same time and it seems an unnecessary complication to the panel.js code. |
It happens with any menu applet, even the default one. Not only that, but having another autohidden panel placed vertically to the same side where the menu is would make the menu open shifted horizontally for the amount of pixels equal to vertical panel's width.
At first sight that'd be correct but thinking thoroughly I could envision an applet that does some automatic work in the background and could be instructed in the code to show a popup menu in certain situation(s) without the user acting upon the applet or the panel itself. This may be the situation for which the panel visibility check is in place. I'm not sure though whether the popup menu display would force the panel to show, or not. |
Using:
instead of:
seems to fix the problem with applet resizing though it doesn't fix the peekPanel() bug I mentioned above. Well. it does some of the time with the menu applet but not all the time strangely. |
The only explanation would be that neither |
@Drugwash2 I think I've found the solution to this in cinnamenu and the menu applet. When the super key is pressed, peekPanel just needs to be called before the menu is toggled open. In menu applet line 1321:
|
Looking at the panel.js code it seems And implementing such workaround in each and every applet hardly seems like the correct solution. Shouldn't this be cared for in the popup menu code itself? I'm not familiar with the code but the principle would be for the popup menu code to actually check for the real panel's visible state (something similar to |
@Drugwash2 peekPanel is only called by an applet when it wants to show the panel so this is up to the applet. For instance, you can turn off the option "force panel to be visible when opening the menu" in the menu applet, in which case peekPanel is not called and the panel is not shown. |
Well, I tried using |
Download and upload speed applet seems to be working fine with this fix (mint 21.2) |
There may have been some improvement in the area between 19.2 (Cinnamon 4.2) and 21.2 (Cinnamon 5.8). Since the 19.x line became irrelevant for most people my test results shouldn't matter much if anything. Hopefully it will keep working correctly on all newer Cinnamon versions. Let's see Gr3q's results with the Weather Applet. |
Ok, it's changed now. The PR fixes most of the issues with positioning at the cost of that if a panel has open menus it will automatically be shown. Download and upload speed must be a special case, and I can take a look at that as well, but the fix probably will be in the applet's code. |
I'll wait for things to settle before testing this fix in Mint 19.2. It feels kinda hacky though to force the panel to be shown even if it wasn't intended to. Now, I don't personally know of any applet that would display a popup menu out of the blue without user interaction with the panel, but nevertheless in theory it could exist. Which means this fix might defeat its purpose. Admittedly it could be treated as a corner case, but too often corner cases are being easily dismissed only for later on to create problems. As for 'download and upload speed', that applet displays the popup menu as a tooltip replacement upon hovering of applet's icon. So in theory the panel should be visible at the time and there would be no need for a fix. Practically though the visible state of the panel seems to be determined much later, at which point the popup menu had already been displayed at the wrong position. It's weird. As if panel visibility is only being detected correctly upon a mouse click (on an applet icon) but not on hover. Because of this annoying issue I had implemented a switch in the settings for a mod of this applet, that would allow the display of a regular tooltip instead of the popup menu, and which seems to work properly. I wonder how the position of a tooltip can be detected so accurately in all situations, while the popup menu's can not. |
The positioning problem comes up when a popup menu resizes after its open. before the panel reported it's not visible but refused to hide If a menu was open, and when the menu repositioned itself on the next resize according to the state the panel reported. |
So practically all that needs done for things to work correctly in all possible scenarios is to make sure the panel always returns its real visible state when queried (use |
@Gr3q What wrong with my fix, it has the advantage of being a one liner? |
Here's what I've been thinking.
Scratch that, it seems it always returns |
And so it seems. I have modified appletGui.js >
|
cd87d7d
to
c037712
Compare
Your fix is ok, I simply missed that comment. I pushed the previous fix to another branch in case we need that behaviour. Also added one more line so panels will recheck it on popupmenu open.
|
@Gr3q Nice. Shall I create a separate PR for the menu applet or should it be included in this one? It's basically just deleting the 3 lines in
and replacing them with the 3 lines in the comment above in Probably better to create a separate PR I'm guessing. |
I think another PR would be better if it still has a timing issue with positioning when opened. |
@Gr3q This commit needs a better description. Also:
This doesn't seem to make any difference. I can add this commit to my PR if it's easier. |
See Weather Applet if the hourly weather button is clicked and the panel is set to auto hide.
The problem is that the panel reports it's not visible even though it is, so the x,y calculation for PopupMenus are incorrect.
Panels are not hidden untilglobal.menuStackLength
is bigger than one (meaning a PopupMenu is open for example)Note I do not know what manipulatesglobal.menuStackLength
so it contains something something else other than applet PopupMenu states this change probably will have unintended side effects. I checked it looks like it's only for PopupMenus.Also we don't know which panels the stackItems belong to, so this will report that all of them are visible. In theory this should not be a problem because you are only able to open only one, right?... but I somehow doubt that.Now this PR has an opinionated decision to show panels that has open popup menus. This seems to be the easiest way to make sure panels report the correct visibility and popupMenus have correct positioning with using both mouse and keyboard bindings to open them.