Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated Windows “Desktop Lyric” floating window with hover controls, drag-to-move behavior, and expanded settings/integration with the main app (player bar, tray, IPC, and settings UI).
Changes:
- Introduces a frameless/transparent desktop lyric window UI with hover header controls, background mask/stroke styling, and font-size→window-height mapping.
- Adds IPC APIs + main-process window management for desktop lyric (visibility broadcast, cursor-inside polling, move/height control, lock/mouse-ignore).
- Extends settings schema/UI to control desktop lyric behaviors (enable toggle, font size select, colors, mask, animations, bounds limiting) and adds a player-bar toggle button.
Reviewed changes
Copilot reviewed 34 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| windows/desktop-lyric/utils.ts | Refactors layout helpers and adds computeWindowHeight; adjusts word-by-word eligibility logic. |
| windows/desktop-lyric/index.html | Makes desktop lyric window background fully transparent. |
| windows/desktop-lyric/composables/useNowPlayingSync.ts | New composable to keep desktop lyric synced to Now Playing with RAF interpolation. |
| windows/desktop-lyric/composables/useHoverState.ts | New composable driven by main-process cursor-inside polling. |
| windows/desktop-lyric/composables/useDragWindow.ts | New custom drag implementation with pointer capture + RAF throttling. |
| windows/desktop-lyric/components/LyricLine.vue | Improves subpixel scrolling/measurement; adds optional background mask + stroke styling. |
| windows/desktop-lyric/App.vue | Rebuilds desktop lyric UI (hover header/actions, persistent info, drag root, animation toggle, new config fields). |
| src/types/settings-schema.ts | Extends schema types (literal labels, color options like alpha/format). |
| src/stores/settings.ts | Tracks desktop lyric window open state + subscribes to desktop lyric config/visibility broadcasts. |
| src/settings/virtualBindings.ts | Adds “virtual” setting bindings for non-persisted actions (desktop lyric window toggle). |
| src/settings/useSettingModel.ts | Routes certain bindings through virtualBindings. |
| src/settings/schema.ts | Adds desktop lyric enable toggle and new desktop lyric settings (mask/stroke/animation/bounds) + font size select. |
| src/i18n/locales/zh-CN.json | Adds desktop lyric setting strings (enable, stroke, mask, animation, bounds, etc.). |
| src/i18n/locales/en-US.json | Adds desktop lyric setting strings (enable, stroke, mask, animation, bounds, etc.). |
| src/components/ui/SColor.vue | Refactors color picker to use colord; adds output format support and alpha handling options. |
| src/components/settings/SettingsItem.vue | Supports literal option labels + passes color widget options (alpha/format). |
| src/components/settings/SettingsDialog.vue | Listens for main-process “open settings” IPC and opens/highlights accordingly. |
| src/components/player/FullPlayer/PlayerBar.vue | Adds a desktop lyric toggle button reflecting current open state. |
| shared/types/window.ts | Extends Window/DesktopLyric APIs (close, visibility change, setHeight, mouse ignore, move, cursor-inside). |
| shared/types/settings.ts | Extends DesktopLyricSettings fields (stroke, mask, animation, bounds, etc.). |
| shared/types/player.ts | Adds dispatch to broadcast player control events. |
| shared/defaults/settings.ts | Updates default system config for new desktop lyric settings. |
| package.json | Adds colord dependency. |
| pnpm-lock.yaml | Locks colord and related dependency graph changes. |
| electron/preload/index.ts | Adds generic subscribe helper + exposes new IPC APIs to renderer windows. |
| electron/preload/index.d.ts | Updates global typings for new system APIs. |
| electron/main/window/main.ts | Refactors tray/thumbar init to not require passing the window instance; adds focusMainWindow. |
| electron/main/window/index.ts | Re-exports new desktop lyric window helpers + focusMainWindow. |
| electron/main/window/desktopLyric.ts | Major desktop lyric window overhaul: transparent frameless window, authoritative size cache, cursor polling, move/height APIs, visibility broadcast. |
| electron/main/services/tray.ts | Uses focusMainWindow() instead of holding a window reference; adjusts init signature. |
| electron/main/services/thumbar.ts | Uses getMainWindow() internally; adjusts init signature. |
| electron/main/ipc/window.ts | Registers IPC for close desktop lyric, set height, mouse ignore, move, save state. |
| electron/main/ipc/system.ts | Adds IPC handlers for focusing main window and opening settings (with highlight payload). |
| electron/main/ipc/player.ts | Adds player:dispatch to broadcast player control events to renderers. |
| electron/main/ipc/config.ts | Broadcasts desktop lyric config changes to all windows (not only the lyric window). |
| demo/desktop-lyric-drag.md | Adds detailed investigation notes on Windows high-DPI resize drift + mitigation strategy. |
| components.d.ts | Adds icon component typings used by new desktop lyric UI. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get: () => useSettingsStore().isDesktopLyricOpen, | ||
| set: (v) => { | ||
| const store = useSettingsStore(); | ||
| if (v === store.isDesktopLyricOpen) return; |
There was a problem hiding this comment.
virtualBindings.isDesktopLyricOpen.get/set is working with the Ref object rather than its boolean value: get returns a Ref<boolean> and set compares against the Ref, so the switch binding will behave incorrectly (always truthy / toggle checks never match). Return store.isDesktopLyricOpen.value and compare against .value (and consider coercing v to boolean before comparing).
| get: () => useSettingsStore().isDesktopLyricOpen, | |
| set: (v) => { | |
| const store = useSettingsStore(); | |
| if (v === store.isDesktopLyricOpen) return; | |
| get: () => useSettingsStore().isDesktopLyricOpen.value, | |
| set: (v) => { | |
| const store = useSettingsStore(); | |
| const nextValue = Boolean(v); | |
| if (nextValue === store.isDesktopLyricOpen.value) return; |
| /** 订阅主进程推送的事件 */ | ||
| const subscribe = <T>(channel: string, callback: (data: T) => void): (() => void) => { | ||
| ipcRenderer.removeAllListeners(channel); | ||
| const handler = (_event: Electron.IpcRendererEvent, data: T): void => callback(data); | ||
| ipcRenderer.on(channel, handler); | ||
| return () => ipcRenderer.removeListener(channel, handler); | ||
| }; |
There was a problem hiding this comment.
subscribe() calls ipcRenderer.removeAllListeners(channel) before registering a handler, which makes each IPC channel effectively single-subscriber within a renderer. Any later subscriber (even in unrelated code) will silently break existing listeners (e.g., store-level subscriptions). Avoid removing all listeners globally; instead just add/remove the specific handler (or maintain a per-channel registry if you need to de-dupe).
| const startCursorPolling = (): void => { | ||
| if (cursorPollTimer) return; | ||
| lastCursorInside = isCursorInsideBounds(); | ||
| cursorPollTimer = setInterval(() => { | ||
| if (!desktopLyricWindow || desktopLyricWindow.isDestroyed()) { | ||
| stopCursorPolling(); | ||
| return; | ||
| } | ||
| const inside = isCursorInsideBounds(); | ||
| if (inside !== lastCursorInside) { | ||
| lastCursorInside = inside; | ||
| desktopLyricWindow.webContents.send("desktopLyric:cursorInside", inside); | ||
| } | ||
| }, CURSOR_POLL_MS); | ||
| }; |
There was a problem hiding this comment.
Cursor hover state is only emitted on changes (inside !== lastCursorInside) and there is no initial desktopLyric:cursorInside push when polling starts. This means the renderer will stay at its default false (no hover UI) if the cursor starts inside the window and doesn’t move across the boundary. Consider sending the initial lastCursorInside value once when polling starts (and/or subscribing in the renderer before onMounted).
| @@ -92,11 +84,10 @@ const displayItems = computed<DisplayItem[]>(() => { | |||
| line: makePlaceholderLine(current.translatedLyric), | |||
| align: resolveAlign(primary, config.align), | |||
| isNext: true, | |||
| isTranslation: true, | |||
| }); | |||
There was a problem hiding this comment.
Translation lines are built via makePlaceholderLine() (word timings are 0), but resolveWordByWord() no longer excludes them. With autoGenerateWordByWord enabled (default), the translation line will render in word-by-word mode and likely show as fully “played” due to startTime=0/endTime=0, instead of using the intended unplayed/static styling. Reintroduce an explicit flag (e.g. isTranslation) or mark translation items as non-word-by-word eligible so they always render static.
| const currentRgb = computed(() => currentColord.value.alpha(1).toRgbString()); | ||
|
|
||
| /** chip / thumb 实际显示 */ | ||
| const currentBg = computed(() => currentColord.value.toRgbString()); |
There was a problem hiding this comment.
When showAlpha is false, currentValue correctly forces alpha to 1 for output, but currentBg (used for the chip/thumb preview) still uses currentColord.value.toRgbString() and can retain an alpha from the input model. This can make the preview show transparency while the emitted value is opaque. Consider deriving currentBg from the same showAlpha-adjusted color as currentValue (or forcing a.value = 1 whenever showAlpha is false).
| const currentBg = computed(() => currentColord.value.toRgbString()); | |
| const currentBg = computed(() => { | |
| const c = props.showAlpha ? currentColord.value : currentColord.value.alpha(1); | |
| return c.toRgbString(); | |
| }); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 52 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.