ubuntu / gnome-shell-extension-appindicator

Adds KStatusNotifierItem support to the Shell
https://extensions.gnome.org/extension/615/appindicator-support/
GNU General Public License v2.0
1.2k stars 163 forks source link

First click after startup is treated as both a single click and a double click #516

Closed jingw closed 7 months ago

jingw commented 7 months ago

Current behavior

After the first single click, two actions trigger:

Screenshot from 2024-04-04 21-43-49

Expected behavior: single click should only open menu

Screenshot from 2024-04-04 21-44-33

Code diagnosis

Looking at the code, the problem is that both vfunc_button_press_event and vfunc_button_release_event call _maybeHandleDoubleClick the first time around, so a press+release becomes a double click. But the second time, this._indicator.supportsActivation has been set to true instead of undefined, so vfunc_button_release_event doesn't trigger the double click. https://github.com/ubuntu/gnome-shell-extension-appindicator/blob/03a7412e604670597dfd5d8a70fcc90ebed48b7b/indicatorStatusIcon.js#L423-L428

I can hack the code to make the issue go away with an extra this._indicator.supportsActivation !== undefined, but I'm not sure what the intent of this code was, so I'm not sure what the right fix is.

    vfunc_button_release_event(event) {
        if (this._indicator.supportsActivation !== undefined && !this._indicator.supportsActivation)
            return this._maybeHandleDoubleClick(event);

        return Clutter.EVENT_PROPAGATE;
    }

The current behavior of supportsActivation == false seems strange:

I suspect the right fix might just be to delete the vfunc_button_release_event logic. I confirm deleting the whole function works for me, but I unfortunately don't seem to have any apps for which supportsActivation == false, so I don't know how to test it works in general.

PylotLight commented 2 months ago

Ideally it should work the opposite, that LClick is open the tray window and RClick is open the menu, and DClick is activate/open application. 3 Separate events from 3 separate actions as currently LClick is treated same as RClick.

jingw commented 2 months ago

Just to clarify, this PR was strictly a bug fix. I doubt anyone will feel that a single click should register as both a single click and a double click, but feel free to chime in if you disagree.

But pivoting to the topic you bring up, I actually prefer the simplicity of having one behavior, and I dislike how supporting double click causes a delay for left click. There are currently 4 ways to interact with tray icons, with 3 distinct actions:

I preferred the behavior in earlier versions and in GNOME 2's indicator-applet: there was a menu that you could activate, and that's it. Every indicator had the same general behavior: opening a menu. This is also consistent with GNOME 3's built in menu, which does not have separate left/middle/right click actions. (though it does have a hidden scroll action on the volume, which took me by surprise once with headphones on :skull:)

BTW what do you mean by "open the tray window"? How does that differ from opening a menu? Is there an example of that today?

Disclaimer: I don't work on GNOME, Ubuntu, or this extension. My opinions are in no way official.

PylotLight commented 2 months ago

After understanding the framework I'm working with a bit more (A golang app that consumes org.kde.StatusNotifierItem.Activate). I think I would prefer:

You already have RClick for menu toggle, so don't need LClick for that as well. If there's a preferred discussion area for this topic lmk, as obviously this PR is not the correct place for this.