feat: disable high power consumption effects on battery #14230
Conversation
|
|
github-actions
bot
added
app:electron
📝 WalkthroughWalkthroughThis change introduces power state monitoring to the Electron app. When the device runs on battery, the system automatically reduces animations and transitions by applying a Changes
Sequence Diagram(s)sequenceDiagram
actor System as System (Battery)
participant Main as Electron Main
participant Renderer as React Renderer
participant DOM as Document DOM
System->>Main: Power state changes (on-battery/on-ac)
Main->>Main: Update isOnBatteryPower state
Renderer->>Main: Query get-power-state via IPC (on mount)
Main-->>Renderer: Return battery status
Renderer->>Renderer: usePowerState hook updates state
Note over Renderer: Check prefers-reduced-motion<br/>media query
Renderer->>Renderer: DesktopPowerStateSync calculates<br/>reduce-motion needed
alt Should Enable Reduce Motion
Renderer->>DOM: Add 'reduce-motion' class
DOM->>DOM: Apply reduce-motion CSS rules
else Should Allow Animations
Renderer->>DOM: Remove 'reduce-motion' class
DOM->>DOM: Restore normal animations
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Fix all issues with AI agents
In @packages/frontend/apps/electron/src/main/index.ts:
- Around line 104-114: The powerMonitor listeners and the initial
isOnBatteryPower read are being registered at module scope before Electron is
ready; wrap the registration and the isOnBatteryPower assignment in
app.whenReady().then(...) so that powerMonitor.on('on-battery', ...),
powerMonitor.on('on-ac', ...), and the isOnBatteryPower =
powerMonitor.isOnBatteryPower() call execute only after app.whenReady()
resolves, keeping the same handler logic and logger.info messages.
🧹 Nitpick comments (1)
packages/frontend/apps/electron-renderer/src/app/global.css (1)
27-35: Consider animation-timing-function and review !important usage.The CSS effectively disables animations, but consider:
Missing property:
animation-timing-functionis not overridden. Whileanimation-duration: 0.01msmakes this mostly moot, explicitly setting it tolinearwould be more thorough.!important usage: The liberal use of
!importantmay be necessary to override inline styles or third-party libraries, but it's worth confirming this is required. If possible, rely on specificity instead.♻️ Suggested refinement
/* Disable animations when on battery power */ html.reduce-motion *, html.reduce-motion *::before, html.reduce-motion *::after { animation-duration: 0.01ms !important; + animation-timing-function: linear !important; animation-iteration-count: 1 !important; transition-duration: 0.01ms !important; scroll-behavior: auto !important; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
.yarn/releases/yarn-4.12.0.cjsis excluded by!**/.yarn/**blocksuite/integration-test/src/__tests__/edgeless/__screenshots__/layer.spec.ts/if-the-topmost-layer-is-canvas-layer--the-length-of-canvasLayers-array-should-equal-to-the-counts-of-canvas-layers-1.pngis excluded by!**/*.pngblocksuite/playground/apps/starter/data/snapshots/affine-default.zipis excluded by!**/*.zippackages/frontend/apps/ios/App/Packages/Intelligents/Sources/Intelligents/Interface/Controller/AttachmentManagementController/AttachmentIcon.xcassets/FileAttachment_json.imageset/AiFileClip attachment icon.pdfis excluded by!**/*.pdfpackages/frontend/apps/ios/App/Packages/Intelligents/Sources/Intelligents/Interface/Controller/AttachmentManagementController/AttachmentIcon.xcassets/FileAttachment_md.imageset/AiFileClip attachment icon.pdfis excluded by!**/*.pdfpackages/frontend/apps/ios/App/Packages/Intelligents/Sources/Intelligents/Interface/Controller/AttachmentManagementController/AttachmentIcon.xcassets/FileAttachment_pdf.imageset/AiFileClip attachment icon.pdfis excluded by!**/*.pdfpackages/frontend/apps/ios/App/Packages/Intelligents/Sources/Intelligents/Interface/Controller/AttachmentManagementController/AttachmentIcon.xcassets/FileAttachment_txt.imageset/AiFileClip attachment icon.pdfis excluded by!**/*.pdf
📒 Files selected for processing (20)
.devcontainer/setup-user.sh.husky/pre-commitpackages/common/y-octo/node/scripts/run-test.mtspackages/frontend/apps/android/App/gradlewpackages/frontend/apps/electron-renderer/src/app/app.tsxpackages/frontend/apps/electron-renderer/src/app/global.csspackages/frontend/apps/electron-renderer/src/app/hooks/use-power-state.tspackages/frontend/apps/electron-renderer/src/app/power-state-sync.tspackages/frontend/apps/electron/scripts/generate-assets.tspackages/frontend/apps/electron/src/main/index.tspackages/frontend/apps/ios/App/App.xcworkspace/xcshareddata/swiftpm/Package.resolvedpackages/frontend/apps/ios/apollo-codegen-chore.shpackages/frontend/apps/ios/setup.shpackages/frontend/apps/ios/update_code_sign_identity.shpackages/frontend/core/public/manifest.jsonscripts/cleanup-canary-releases.shscripts/set-version.shtools/cli/bin/cli.jstools/cli/bin/runner.jstools/cli/src/init.ts
💤 Files with no reviewable changes (1)
- packages/frontend/apps/ios/App/App.xcworkspace/xcshareddata/swiftpm/Package.resolved
🧰 Additional context used
🧬 Code graph analysis (2)
packages/frontend/apps/electron-renderer/src/app/power-state-sync.ts (1)
packages/frontend/apps/electron-renderer/src/app/hooks/use-power-state.ts (1)
usePowerState(3-11)
packages/frontend/apps/electron-renderer/src/app/app.tsx (1)
packages/frontend/apps/electron-renderer/src/app/power-state-sync.ts (1)
DesktopPowerStateSync(5-38)
🔇 Additional comments (2)
packages/frontend/apps/electron-renderer/src/app/app.tsx (1)
14-14: LGTM: Component integration follows established patterns.The placement and usage of
DesktopPowerStateSyncis consistent with other sync components (DesktopThemeSync,DesktopLanguageSync) in the component tree.Also applies to: 52-52
packages/frontend/apps/electron-renderer/src/app/power-state-sync.ts (1)
5-38: Component logic is sound, but depends on fixing the power state hook.The synchronization logic correctly combines battery state and user preferences:
- Animations are enabled only when NOT on battery AND user does NOT prefer reduced motion
- The
reduce-motionclass is applied when either condition requires it- Cross-browser compatibility for media query listeners is well-handled
However, this component's effectiveness is blocked by the critical issue in
usePowerState(packages/frontend/apps/electron-renderer/src/app/hooks/use-power-state.ts), which doesn't update when power state changes at runtime.Optional optimization:
The media query could be memoized to avoid recreating it on every effect run:
♻️ Minor optimization
+import { useEffect, useMemo, useState } from 'react'; -import { usePowerState } from './hooks/use-power-state'; +import { usePowerState } from './hooks/use-power-state'; export const DesktopPowerStateSync = () => { const onBattery = usePowerState(); + const mediaQuery = useMemo( + () => window.matchMedia('(prefers-reduced-motion: reduce)'), + [] + ); const [prefersReducedMotion, setPrefersReducedMotion] = useState( - () => window.matchMedia('(prefers-reduced-motion: reduce)').matches + () => mediaQuery.matches ); useEffect(() => { - const mediaQuery = window.matchMedia('(prefers-reduced-motion: reduce)'); const handleChange = (e: MediaQueryListEvent) => { setPrefersReducedMotion(e.matches); }; // Modern browsers if (mediaQuery.addEventListener) { mediaQuery.addEventListener('change', handleChange); return () => mediaQuery.removeEventListener('change', handleChange); } // Fallback for older browsers else if (mediaQuery.addListener) { mediaQuery.addListener(handleChange); return () => mediaQuery.removeListener(handleChange); } - }, []); + }, [mediaQuery]); useEffect(() => { const enableAnimations = !onBattery && !prefersReducedMotion; document.documentElement.classList.toggle( 'reduce-motion', !enableAnimations ); }, [onBattery, prefersReducedMotion]); return null; };
| export function usePowerState() { | ||
| const [onBattery, setOnBattery] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| window.__apis.getPowerState().then(setOnBattery); | ||
| }, []); | ||
|
|
||
| return onBattery; | ||
| } |
There was a problem hiding this comment.
Critical: Hook doesn't react to power state changes.
This hook queries the power state only once on mount and never updates. Combined with the main process not broadcasting power state changes, this means:
- If the user unplugs/plugs in their device after mount, the UI won't respond
- The reduce-motion behavior will remain stale until the app restarts
- The battery-saving feature is effectively broken for runtime power changes
Additionally, there's no error handling for the IPC call, which could cause the component to silently fail.
🔧 Proposed fix: Add event listener for power state updates
import { useEffect, useState } from 'react';
export function usePowerState() {
const [onBattery, setOnBattery] = useState(false);
useEffect(() => {
- window.__apis.getPowerState().then(setOnBattery);
- }, []);
+ // Get initial state
+ window.__apis.getPowerState()
+ .then(setOnBattery)
+ .catch(err => {
+ console.error('Failed to get power state:', err);
+ });
+
+ // Listen for power state changes
+ const handlePowerStateChange = (_event: any, onBattery: boolean) => {
+ setOnBattery(onBattery);
+ };
+
+ window.electron?.ipcRenderer.on('power-state-changed', handlePowerStateChange);
+
+ return () => {
+ window.electron?.ipcRenderer.removeListener('power-state-changed', handlePowerStateChange);
+ };
+ }, []);
return onBattery;
}Note: This requires the main process changes suggested in packages/frontend/apps/electron/src/main/index.ts to broadcast power state changes.
Committable suggestion skipped: line range outside the PR's diff.
| powerMonitor.on('on-battery', () => { | ||
| isOnBatteryPower = true; | ||
| logger.info('Running on battery'); | ||
| }); | ||
|
|
||
| powerMonitor.on('on-ac', () => { | ||
| isOnBatteryPower = false; | ||
| logger.info('Running on AC power'); | ||
| }); | ||
|
|
||
| isOnBatteryPower = powerMonitor.isOnBatteryPower(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Electron powerMonitor API requirements app ready event listeners
💡 Result:
Short answer
- powerMonitor is a main-process API and cannot be required or used until app has emitted the ready event. Use app.on('ready', …) or app.whenReady() (or check app.isReady()) before attaching listeners. [1][2]
Example
- Recommended pattern:
const { app, powerMonitor } = require('electron')
app.whenReady().then(() => {
powerMonitor.on('suspend', () => { console.log('suspend') })
powerMonitor.on('resume', () => { console.log('resume') })
})
Notes
- powerMonitor emits events like 'suspend', 'resume', 'on-ac', 'on-battery' (Windows) and 'shutdown' (macOS/Linux). [1]
- Put all powerMonitor usage in the main process after ready to avoid runtime errors. [1][2]
Sources
[1] Electron powerMonitor docs.
[2] Electron app lifecycle (ready / whenReady / isReady).
🏁 Script executed:
# First, let's examine the actual file to understand the code context
head -120 packages/frontend/apps/electron/src/main/index.ts | tail -50Repository: toeverything/AFFiNE
Length of output: 1126
🏁 Script executed:
# Get more context to see app.whenReady() calls and overall structure
cat -n packages/frontend/apps/electron/src/main/index.ts | head -130Repository: toeverything/AFFiNE
Length of output: 5290
🏁 Script executed:
# Check if app.whenReady() is used elsewhere in the file
rg "app\.whenReady" packages/frontend/apps/electron/src/main/index.tsRepository: toeverything/AFFiNE
Length of output: 45
Wrap powerMonitor listeners in app.whenReady() before registration.
The Electron powerMonitor API cannot be used until the app has emitted the ready event. Currently, the event listeners at lines 104-114 are registered at module scope during initialization, which executes before app.whenReady() resolves. This violates Electron's API requirements and will cause runtime errors.
Move these listeners into an app.whenReady().then() block:
Example fix:
app.whenReady().then(() => {
powerMonitor.on('on-battery', () => {
isOnBatteryPower = true;
logger.info('Running on battery');
});
powerMonitor.on('on-ac', () => {
isOnBatteryPower = false;
logger.info('Running on AC power');
});
isOnBatteryPower = powerMonitor.isOnBatteryPower();
});🤖 Prompt for AI Agents
In @packages/frontend/apps/electron/src/main/index.ts around lines 104 - 114,
The powerMonitor listeners and the initial isOnBatteryPower read are being
registered at module scope before Electron is ready; wrap the registration and
the isOnBatteryPower assignment in app.whenReady().then(...) so that
powerMonitor.on('on-battery', ...), powerMonitor.on('on-ac', ...), and the
isOnBatteryPower = powerMonitor.isOnBatteryPower() call execute only after
app.whenReady() resolves, keeping the same handler logic and logger.info
messages.
| ipcMain.handle('get-power-state', () => { | ||
| return isOnBatteryPower; | ||
| }); |
There was a problem hiding this comment.
Critical: Power state changes are not propagated to the renderer.
The IPC handler only returns the current state when queried, but there's no mechanism to notify the renderer when power state changes. This means:
- The renderer's
usePowerStatehook queries once on mount and never updates - If a user unplugs their device after the app loads, the UI will continue showing AC-powered behavior
- The
on-batteryandon-acevent handlers updateisOnBatteryPowerbut don't send events to the renderer
💡 Proposed fix: Add event broadcasting to renderer
+/**
+ * Broadcast power state changes to all renderer windows
+ */
+function broadcastPowerStateChange(onBattery: boolean) {
+ const { BrowserWindow } = require('electron');
+ BrowserWindow.getAllWindows().forEach(window => {
+ window.webContents.send('power-state-changed', onBattery);
+ });
+}
+
powerMonitor.on('on-battery', () => {
isOnBatteryPower = true;
logger.info('Running on battery');
+ broadcastPowerStateChange(true);
});
powerMonitor.on('on-ac', () => {
isOnBatteryPower = false;
logger.info('Running on AC power');
+ broadcastPowerStateChange(false);
});Then update the renderer's usePowerState hook to listen for these events.
Committable suggestion skipped: line range outside the PR's diff.
|
View your CI Pipeline Execution ↗ for commit ef52886
☁️ Nx Cloud last updated this comment at |
2 participants
Fixes#5864
What this PR does-----------------------------------
#Why------------------------------------------------
This improves battery life and aligns the UI behavior with system power conditions.
How it works---------------------------
powerMonitorto detect power state#Checklist-------------------------
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.