Tray menu: Update only on demand #4990 #4985 #5072
Conversation
| @@ -699,8 +709,6 @@ void ownCloudGui::slotUpdateProgress(const QString &folder, const ProgressInfo& | |||
| _recentItemsActions.takeFirst()->deleteLater(); | |||
| } | |||
| _recentItemsActions.append(action); | |||
|
|
|||
| slotRebuildRecentMenus(); | |||
There was a problem hiding this comment.
If this is removed, how is _recentActionsMenu now updated while the menu is shown during sync?
There was a problem hiding this comment.
Yes this is a mistake, it should be called when the menu is visible!
|
Hm, I could try this... provided compiling and running owncloud from source somewhere in $HOME is not too difficult. ;) Should I wait until this is merged (since it seems to have other problems still)? |
|
What's happening here? |
|
@danimo Is your question about the contents of the patch or about "when will this be merged?" Now that I'm back from vacation, I'll update this patch and then it can be merged. |
ckamm
force-pushed
the
contextmenu_on_demand
branch
from
13cf75e
to
3e27fd0
Compare
|
@jturcotte Does it look good to you now? |
| @@ -552,6 +558,11 @@ void ownCloudGui::setupContextMenu() | |||
| } | |||
| } | |||
|
|
|||
| void ownCloudGui::setupContextMenuIfVisible() | |||
| { | |||
| if (!_contextMenu || _contextMenu->isVisible()) | |||
There was a problem hiding this comment.
Is it correct to enter the "if visible" code path when !_contextMenu?
There was a problem hiding this comment.
Hm, I added this with intent and it does work: setupContextMenu() builds the context menu if necessary. But one might just as well argue that setting up the context menu when it's not being shown yet is pointless. And _contextMenu && would be less confusing to read. I'll update accordingly.
The tray menu is now only updated when it becomes visible or while it is visible.
ckamm
force-pushed
the
contextmenu_on_demand
branch
from
3e27fd0
to
b22f9fa
Compare
| @@ -552,6 +558,11 @@ void ownCloudGui::setupContextMenu() | |||
| } | |||
| } | |||
|
|
|||
| void ownCloudGui::setupContextMenuIfVisible() | |||
| { | |||
| if (_contextMenu && _contextMenu->isVisible()) | |||
There was a problem hiding this comment.
For platform tray icons, _contextMenu->isVisible() will always return false.
Though I am not saying that this change is wrong. I am going to cherry-pick it to the Ubuntu packaging anyway, because without it the context menu is updated every 30 seconds, and Qt quickly runs out of items IDs.
There was a problem hiding this comment.
will always return false.
isn't that a bug?
I remember there was some follow up work by me and@ckamm to set our own visible variable to true and false when showing or hidding menu. But maybe we factored that out again
The tray menu is now only updated when it becomes visible or while
it is visible.