Add feature: auto hide controls island#1534
Conversation
⏳ Approval required for deploying to Cloudflare Workers (Preview) for stage-web.
Hey, @nekomeowww, @sumimakito, @luoling8192, @LemonNekoGH, kindly take some time to review and approve this deployment when you are available. Thank you! 🙏 |
There was a problem hiding this comment.
Code Review
This pull request introduces several significant updates, including a version bump to 0.9.0-beta.2, the addition of a power monitor service for Electron, and a new expression controller for Live2D models. It also refactors the speech pipeline to support concurrent TTS generation and implements a consumer registration system for event delivery. My feedback focuses on potential performance and architectural concerns: the global Cache-Control middleware may be too broad, the speech pipeline's concurrency management could be optimized, and the consumer selection logic in the server runtime should be monitored for scalability as the number of connected modules increases.
| .use('*', async (c, next) => { | ||
| await next() | ||
|
|
||
| // NOTICE: All API responses should be non-cacheable. Auth responses can | ||
| // carry session state through redirects, and stale API payloads are not | ||
| // safe to serve from edge caches after user/account mutations. | ||
| c.res.headers.set('Cache-Control', 'no-store, no-cache, private, max-age=0') | ||
| c.res.headers.set('Pragma', 'no-cache') | ||
| c.res.headers.set('Expires', '0') | ||
| }) |
| while (!intent.controller.signal.aborted && inFlightTasks.size >= ttsMaxConcurrent) { | ||
| await Promise.race(inFlightTasks) | ||
| } |
| const selectedConsumer = selectConsumer(event, peer.id, delivery) | ||
| if (delivery && (delivery.mode === 'consumer' || delivery.mode === 'consumer-group')) { | ||
| if (!selectedConsumer) { | ||
| logger.withFields({ peer: peer.id, peerName: p.name, event, delivery }).warn('no consumer registered for event delivery') | ||
| if (delivery.required) { | ||
| send(peer, RESPONSES.error(ServerErrorMessages.noConsumerRegistered, instanceId, event.metadata?.event.id)) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| logger.withFields({ | ||
| fromPeer: peer.id, | ||
| fromPeerName: p.name, | ||
| toPeer: selectedConsumer.peer.id, | ||
| toPeerName: selectedConsumer.name, | ||
| event, | ||
| delivery, | ||
| }).debug('sending event to selected consumer') | ||
|
|
||
| selectedConsumer.peer.send(payload) | ||
| } | ||
| catch (err) { | ||
| logger.withFields({ | ||
| fromPeer: peer.id, | ||
| fromPeerName: p.name, | ||
| toPeer: selectedConsumer.peer.id, | ||
| toPeerName: selectedConsumer.name, | ||
| event, | ||
| delivery, | ||
| }).withError(err).error('failed to send event to selected consumer, removing peer') | ||
|
|
||
| peers.delete(selectedConsumer.peer.id) | ||
| unregisterModulePeer(selectedConsumer, 'consumer send failed') | ||
| } | ||
| return | ||
| } |
6a149c7 to
c6035c9
Compare
Add configurable auto-hide feature for controls island: - autoHideControlsIsland: toggle to enable/disable - autoHideDelay: delay before hiding (0-60s, default 5s) - autoShowDelay: delay before showing after mouse enters (0-5s, default 0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c6035c9 to
a2b65bc
Compare
There was a problem hiding this comment.
Code Review
This pull request implements an auto-hide feature for the controls island, allowing users to configure show/hide delays and hidden opacity levels. The changes include refactoring the settings store, adding unit tests, and updating the settings UI. Feedback highlights a logic flaw where the island collapses immediately upon mouse exit, disregarding the configured delay, and suggests utilizing refDebounced for a more robust implementation. Additionally, the reviewer pointed out several hardcoded strings in the UI that should be migrated to i18n translation keys to maintain localization standards.
| watch(isOutside, (outside) => { | ||
| lastUpdateTime = Date.now() | ||
| timeSinceOutside.value = 0 | ||
| timeSinceInside.value = 0 | ||
|
|
||
| if (outside && expanded.value && !isBlocked.value) { | ||
| expanded.value = false | ||
| } |
There was a problem hiding this comment.
此处的 watch 逻辑在鼠标移出时立即将 expanded 设为 false,这会导致菜单立即折叠,忽略了用户设置的 autoHideDelay 延迟。建议移除此处的立即折叠逻辑,并恢复使用 refDebounced 来实现符合预期的延迟折叠行为。
watch(isOutside, () => {
lastUpdateTime = Date.now()
timeSinceOutside.value = 0
timeSinceInside.value = 0
})
const isOutsideAfterDelay = refDebounced(isOutside, autoHideDelayMs)
watch(isOutsideAfterDelay, (outside) => {
if (outside && expanded.value && !isBlocked.value) {
expanded.value = false
}
})
| import { useSettings, useSettingsAudioDevice, useSettingsControlsIsland } from '@proj-airi/stage-ui/stores/settings' | ||
| import { useTheme } from '@proj-airi/ui' | ||
| import { refDebounced, useIntervalFn } from '@vueuse/core' | ||
| import { useIntervalFn } from '@vueuse/core' |
| expanded.value = false | ||
| } | ||
| }, 1500) | ||
| }, () => autoHideDelayMs.value || 5000) |
| label="Auto-hide controls island" | ||
| description="Hide controls island after mouse leaves, show after mouse enters" |
| :max="5" | ||
| :step="0.1" | ||
| :format-value="v => `${v}s`" | ||
| label="Hide delay" |
| :max="5" | ||
| :step="0.1" | ||
| :format-value="v => `${v}s`" | ||
| label="Show delay" |
| :max="100" | ||
| :step="1" | ||
| :format-value="v => `${v}%`" | ||
| label="Hidden opacity" |
1f30a0d to
b944bd8
Compare
…or improved delay handling
…les for improved UI behavior
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements auto-hide and auto-collapse functionality for the controls island, replacing manual debounce logic with dedicated composables (useControlsIslandAutohide and useControlsIslandCollapse). The settings store and UI are updated to support configurable hide/show delays and opacity, while the previous icon size setting is removed in favor of static styling. Review feedback recommends using internationalization for new UI labels and exposing the new settings in the main settings store for consistency.
| label="Auto-hide controls island" | ||
| description="Hide controls island after mouse leaves, show after mouse enters" |
There was a problem hiding this comment.
这里的 label 和 description 使用了硬编码的英文文本。建议使用 t() 函数并引用语言包中的键值。但请注意,如果这些硬编码值是临时性的且未来可能独立修改,根据项目规则,建议保持它们独立以方便后续调整,避免过早进行抽象。
References
- Avoid abstracting duplicated hardcoded values into a constant if they are temporary and intended to be replaced individually. Keeping them separate can improve visibility and simplify future modifications.
| allowVisibleOnAllWorkspaces: controlsIslandRefs.allowVisibleOnAllWorkspaces, | ||
| alwaysOnTop: controlsIslandRefs.alwaysOnTop, |
There was a problem hiding this comment.
虽然 useSettings 已被标记为弃用,但作为统一的设置入口,建议也将新增加的自动隐藏相关设置项(autoHideControlsIsland, autoHideDelay, autoShowDelay, autoHideOpacity)同步暴露出来,以保持接口的一致性。在处理这些 UI 隐藏相关的设置时,请注意根据背景类型进行审美适配。
| allowVisibleOnAllWorkspaces: controlsIslandRefs.allowVisibleOnAllWorkspaces, | |
| alwaysOnTop: controlsIslandRefs.alwaysOnTop, | |
| allowVisibleOnAllWorkspaces: controlsIslandRefs.allowVisibleOnAllWorkspaces, | |
| alwaysOnTop: controlsIslandRefs.alwaysOnTop, | |
| autoHideControlsIsland: controlsIslandRefs.autoHideControlsIsland, | |
| autoHideDelay: controlsIslandRefs.autoHideDelay, | |
| autoShowDelay: controlsIslandRefs.autoShowDelay, | |
| autoHideOpacity: controlsIslandRefs.autoHideOpacity, |
References
- Conditionally hide UI elements that clash aesthetically with certain background types, such as custom images.
…rolsIslandCollapse composables
…amline timeout function
…ed useSettings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an auto-hide and auto-collapse feature for the Tamagotchi stage controls island. It includes new Pinia store settings for hide/show delays and opacity, two new composables (useControlsIslandAutoHide and useControlsIslandCollapse) to manage the logic, and corresponding UI updates in the settings panel. A potential issue was identified in the auto-hide logic where rapid mouse movements could cause visual flickering due to immediate state resets in the watcher; a refactoring of the timer logic is suggested to ensure smoother transitions.
| const isOutsideDelayed = ref(isOutside.value) | ||
| const isInsideDelayed = ref(!isOutside.value) | ||
|
|
||
| // --- Auto-hide island (only works when autoHideControlsIsland = true) --- | ||
| const { start: startOutside, stop: stopOutside } = useTimeoutFn(() => { | ||
| isOutsideDelayed.value = true | ||
| }, () => autoHideDelay.value * 1000, { immediate: false }) | ||
|
|
||
| const { start: startInside, stop: stopInside } = useTimeoutFn(() => { | ||
| isInsideDelayed.value = true | ||
| }, () => autoShowDelay.value * 1000, { immediate: false }) | ||
|
|
||
| // Watch mouse position changes | ||
| watch(isOutside, (val) => { | ||
| if (!autoHideControlsIsland.value) { | ||
| // Not in auto-hide mode, reset states | ||
| stopOutside() | ||
| stopInside() | ||
| isOutsideDelayed.value = val | ||
| isInsideDelayed.value = !val | ||
| return | ||
| } | ||
| stopOutside() | ||
| stopInside() | ||
| if (val) { | ||
| // Mouse left - start hide delay timer | ||
| isInsideDelayed.value = false | ||
| startOutside() | ||
| } | ||
| else { | ||
| // Mouse entered - start show delay timer | ||
| isOutsideDelayed.value = false | ||
| startInside() | ||
| } | ||
| }) |
There was a problem hiding this comment.
The current implementation of the auto-hide logic can cause the island to flicker (briefly hide and then show) when the mouse moves quickly out and back into the island area. This happens because isInsideDelayed and isOutsideDelayed are reset immediately in the isOutside watcher, which affects the isHidden computed property before the timers have a chance to fire.
Refactoring the logic to only update these states when the respective delays are met (or when the feature is disabled) provides a smoother user experience and avoids unnecessary state transitions during quick mouse movements.
const isOutsideDelayed = ref(isOutside.value)
const isInsideDelayed = ref(!isOutside.value)
// --- Auto-hide island (only works when autoHideControlsIsland = true) ---
const { start: startOutside, stop: stopOutside } = useTimeoutFn(() => {
isOutsideDelayed.value = true
isInsideDelayed.value = false
}, () => autoHideDelay.value * 1000, { immediate: false })
const { start: startInside, stop: stopInside } = useTimeoutFn(() => {
isInsideDelayed.value = true
isOutsideDelayed.value = false
}, () => autoShowDelay.value * 1000, { immediate: false })
// Watch mouse position changes
watch(isOutside, (val) => {
stopOutside()
stopInside()
if (!autoHideControlsIsland.value) {
// Not in auto-hide mode, reset states
isOutsideDelayed.value = val
isInsideDelayed.value = !val
return
}
if (val) {
// Mouse left - start hide delay timer if it was shown
if (isInsideDelayed.value)
startOutside()
}
else {
// Mouse entered - start show delay timer if it was hidden
if (isOutsideDelayed.value)
startInside()
}
})Only update delayed states when the respective delays are met (or when delay is 0), avoiding unnecessary state transitions during rapid mouse movements in/out of the island area. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Description
Linked Issues
#1405
Additional Context