feat(stage-tamagotchi): add always-on-top toggle IPC API#1201
feat(stage-tamagotchi): add always-on-top toggle IPC API#1201JasonOA888 wants to merge 2 commits intomoeru-ai:mainfrom
Conversation
Implements moeru-ai#1159 Added IPC handlers for toggling always-on-top: - window:set-always-on-top(enabled): Toggle always-on-top - window:get-always-on-top(): Get current state Default: enabled (current behavior) Note: UI toggle to be added in separate commit
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Inter-Process Communication (IPC) API to manage the "always-on-top" feature of the application window. This enhancement provides a programmatic way for the renderer process to toggle and query the window's always-on-top status, laying the groundwork for future user interface controls without altering the current default behavior. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an IPC API to toggle the always-on-top state of the main window. The implementation is functional, and my review includes suggestions to improve robustness and maintainability. Specifically, I've recommended removing redundant state management in the main process in favor of using the BrowserWindow instance as the single source of truth, and adding explicit types to the new preload API for better type safety in the renderer process, along with defining IPC channel names as constants.
| let alwaysOnTopEnabled = true // default enabled | ||
| window.setAlwaysOnTop(alwaysOnTopEnabled, 'screen-saver', 1) | ||
|
|
||
| // IPC handler for toggling always-on-top | ||
| ipcMain.handle('window:set-always-on-top', (_event, enabled: boolean) => { | ||
| alwaysOnTopEnabled = enabled | ||
| window.setAlwaysOnTop(enabled, 'screen-saver', 1) | ||
| return enabled | ||
| }) | ||
|
|
||
| ipcMain.handle('window:get-always-on-top', () => { | ||
| return alwaysOnTopEnabled | ||
| }) |
There was a problem hiding this comment.
To improve robustness and simplify the code, it's better to avoid maintaining a separate state variable for the 'always-on-top' status. The BrowserWindow instance should be the single source of truth. You can use window.isAlwaysOnTop() in the get handler to directly query the window's state. This eliminates the alwaysOnTopEnabled variable and prevents any potential for state desynchronization.
// Always on top - can be toggled via IPC
window.setAlwaysOnTop(true, 'screen-saver', 1)
// IPC handler for toggling always-on-top
ipcMain.handle('window:set-always-on-top', (_event, enabled: boolean) => {
window.setAlwaysOnTop(enabled, 'screen-saver', 1)
return enabled
})
ipcMain.handle('window:get-always-on-top', () => {
return window.isAlwaysOnTop()
})| export const alwaysOnTopAPI = { | ||
| set: (enabled: boolean) => ipcRenderer.invoke('window:set-always-on-top', enabled), | ||
| get: () => ipcRenderer.invoke('window:get-always-on-top'), | ||
| } |
There was a problem hiding this comment.
For better type safety and maintainability, it's good practice to define an explicit interface for the exposed API. This helps consumers in the renderer process understand the method signatures and return types, enabling better autocompletion and compile-time checks.
Additionally, the IPC channel names are hardcoded as strings. It is recommended to define them as constants in a shared location to prevent typos and ease future maintenance.
export interface AlwaysOnTopAPI {
set: (enabled: boolean) => Promise<boolean>;
get: () => Promise<boolean>;
}
export const alwaysOnTopAPI: AlwaysOnTopAPI = {
set: (enabled: boolean) => ipcRenderer.invoke('window:set-always-on-top', enabled),
get: () => ipcRenderer.invoke('window:get-always-on-top'),
}- Use window.isAlwaysOnTop() as single source of truth (no state variable) - Add AlwaysOnTopAPI interface for type safety - Remove redundant alwaysOnTopEnabled variable Thanks @gemini-code-assist for the review!
|
#1183 already implemented. |
2 participants
Implements #1159
Changes
Added IPC API for toggling always-on-top mode:
Main Process
window:set-always-on-top(enabled)- Toggle always-on-topwindow:get-always-on-top()- Get current statePreload
window.alwaysOnTop.set(enabled)- Set modewindow.alwaysOnTop.get()- Get modeDefault
Enabled (current behavior preserved)
Usage
Note: UI toggle button can be added separately.