[V3-Linux] Systray OnClick on initial icon click #3907
Conversation
WalkthroughThe pull request introduces significant updates to the project's changelog, detailing new features such as support for Darwin universal builds, documentation for events, and enhancements to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
- Convert event handling to switch statement for better readability - Fix menu event handlers to properly trigger open/close callbacks - Update click behavior to use doubleClickHandler for Activate CHANGELOG.md
atterpac
force-pushed
the
v3/linux-systray-fixes
branch
from
12c61d4
to
6b5d0f9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
v3/examples/systray/main.go (1)
95-105: Enhance example to demonstrate best practicesAs this is an example file that developers will reference, it should demonstrate:
- Proper platform-specific handling (especially Linux D-Bus integration)
- Proper logging practices
- Error handling
- Documentation of the platform-specific behaviors
This will help developers understand the correct implementation patterns for different platforms.
Consider adding comments explaining the platform-specific behaviors and requirements, particularly for Linux:
- Initial click handling via com.canonical
- Double-click handling via StatusNotifier
- Right-click behavior
v3/pkg/application/systemtray_linux.go (2)
424-424: Implement menu opening using com.canonical D-Bus messagesThe FIXME comment suggests using com.canonical for menu opening, which aligns with the PR objectives. Consider implementing this functionality instead of just logging that it's not implemented.
Would you like help implementing the menu opening functionality using com.canonical D-Bus messages?
712-712: Consider implementing context menu functionalityWhile the explicit return statement improves code clarity, the function could benefit from actual implementation of context menu functionality, especially since the PR mentions that
OnRightClick()is functioning correctly.Consider implementing the context menu functionality to properly handle right-click events, possibly by showing the menu at the provided coordinates (x, y).
mkdocs-website/docs/en/changelog.md (3)
35-35: Enhance the changelog entry to better reflect the technical improvements.The current entry could be expanded to better capture the technical details and user impact. Consider this revision:
-- Refactored systray click messaging to better align with user interactions by @atterpac in [#3907](https://github.com/wailsapp/wails/pull/3907) +- [linux] Enhanced systray click handling by leveraging com.canonical D-Bus messages for initial clicks and StatusNotifier's Activate signal for double clicks, providing more intuitive user interaction by @atterpac in [#3907](https://github.com/wailsapp/wails/pull/3907)This revision:
- Specifies the platform affected [linux]
- Details the technical implementation (D-Bus messages, StatusNotifier)
- Clarifies the user experience improvement
Line range hint
93-94: Remove duplicate entries.These entries appear twice in the changelog:
- Do not bind internal service methods in [#3720](https://github.com/wailsapp/wails/pull/3720) by [leaanthony](https://github.com/leaanthony) - [windows] Fixed system tray startup panic in [#3693](https://github.com/wailsapp/wails/issues/3693) by [@DeltaLaboratory](https://github.com/DeltaLaboratory)
Line range hint
1-24: Maintain consistent platform indicator formatting.The changelog uses both styles for platform indicators:
[platform](e.g., "[linux]")(platform)(e.g., "(Darwin)")Consider standardizing to the
[platform]format throughout the document for better readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
mkdocs-website/docs/en/changelog.md(1 hunks)v3/examples/systray/main.go(1 hunks)v3/pkg/application/systemtray_linux.go(3 hunks)
🔇 Additional comments (2)
v3/pkg/application/systemtray_linux.go (2)
625-637: LGTM: Clean event handling implementation
The switch statement implementation improves code readability and correctly handles menu events. The changes align well with the PR objectives for refactoring event handling.
703-705: LGTM: Correct implementation of double-click handling
The change to use doubleClickHandler instead of clickHandler in the Activate method correctly implements the behavior described in the PR objectives, addressing the initial click handling issue.
Linux systray has some quirks this PR hopes to remedy
someof them.The StatusNotifier
Activatesignal emits when the systray is clicked for the first time. However this is the first click once the tray has been focused and is active, thus really being a double click for the end user once to focus the tray and another to get the "first" clickHowever
com.canonicalemits dbus messages when the systray is clicked initially and we already have that dbus setup to handle menus we just tie it into the systray and now we havesystemTray.OnClick()firing from the canonical menu dbus emit andsystemTray.OnDoubleClickfiring based off of the StatusNotifier "Activate" message.systemTray.OnRightClick()is functioning in my OS as wellChanges:
Summary by CodeRabbit
New Features
Bug Fixes
AlwaysOnTopfeature on Mac and service name determination.Improvements