[Feat] 순공시간 타이머 동작 시 사용성 개선#62
Hidden character warning
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough네이티브 브리지로 WebView의 타이머 활성/비활성 메시지를 처리하고, 화면 밝기(dim) 조절·화면 꺼짐 방지(keep-awake)를 담당하는 timer display mode 서비스와 연동하도록 WebView 라우트·라이프사이클·하드웨어 백 처리 및 Expo 의존성·앱 버전 식별자를 업데이트했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant WebView as WebView
participant NativeBridge as NativeBridge
participant TimerService as TimerDisplayMode 서비스
participant ExpoKeepAwake as Expo Keep-Awake
participant ExpoBrightness as Expo Brightness
WebView->>NativeBridge: postMessage "TIMER_ACTIVE" {keepAwake, dimScreen, brightnessLevel}
NativeBridge->>NativeBridge: origin 검증 (getUrlOrigin)
NativeBridge->>TimerService: enableTimerDisplayMode(options)
TimerService->>ExpoKeepAwake: activateKeepAwakeAsync (if keepAwake)
TimerService->>ExpoBrightness: setBrightnessAsync / save original (if dimScreen)
WebView->>NativeBridge: postMessage "TIMER_INACTIVE"
NativeBridge->>TimerService: disableTimerDisplayMode()
TimerService->>ExpoBrightness: restore original brightness
TimerService->>ExpoKeepAwake: deactivateKeepAwake()
WebView->>NativeBridge: postMessage "NAVIGATE_BACK"
NativeBridge->>WebView: inject JS 또는 goBack() (origin 및 canGoBack 조건에 따라)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
타이머(순공시간) 동작 중 화면 꺼짐/뒤로가기 등으로 흐름이 끊기는 문제를 줄이기 위해, 네이티브 레이어에서 밝기/keep-awake 및 뒤로가기 제어를 추가한 PR입니다.
Changes:
expo-brightness,expo-keep-awake추가 및 Expo 관련 패키지 버전 정리- WebView 브리지 메시지에 타이머 활성/비활성 및 뒤로가기 제어(
TIMER_ACTIVE,TIMER_INACTIVE,NAVIGATE_BACK) 추가 - 타이머 활성 시 밝기/keep-awake 적용, 백그라운드 전환 및 언마운트 시 원복 처리
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
services/timerDisplayMode.ts |
타이머 집중 모드(밝기 조절, keep-awake) enable/disable 유틸 추가 |
app/webview/[path].tsx |
WebView 브리지 메시지 확장, 타이머 중 iOS 제스처/Android 백 버튼 동작 변경, AppState 연동 |
package.json |
expo-brightness, expo-keep-awake 추가 및 Expo 패키지 버전 조정 |
pnpm-lock.yaml |
의존성 추가/업데이트에 따른 lockfile 갱신 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| '@expo/router-server@55.0.11': | ||
| resolution: {integrity: sha512-Kd8J1OOlFR00DZxn+1KfiQiXZtRut6cj8+ynqHJa7dtt/lTL4tGkYistqmVhpKJ6w886eRY5WivKy7o0ZBFkJA==} | ||
| peerDependencies: | ||
| '@expo/metro-runtime': 5.0.4 | ||
| '@expo/metro-runtime': ^55.0.6 | ||
| expo: '*' | ||
| expo-constants: ^55.0.9 | ||
| expo-font: ^55.0.4 |
There was a problem hiding this comment.
@expo/router-server@55.0.11 now declares a peer dependency on @expo/metro-runtime: ^55.0.6, but this lockfile still resolves @expo/metro-runtime@5.0.4 (and there are no @expo/metro-runtime@55.x entries). This can lead to persistent peer-dependency warnings or an unintended runtime mismatch if router-server actually expects a different metro-runtime major. Consider re-syncing Expo package versions via expo install (or otherwise ensuring @expo/router-server and @expo/metro-runtime are on compatible version ranges).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/webview/[path].tsx (1)
93-100:⚠️ Potential issue | 🔴 Critical
startsWith기반 origin 검증은 우회 가능합니다.
event.nativeEvent.url은 전체 URL이기 때문에https://allowed.example.evil/path같은 주소도 현재 검사에 통과합니다. 이 상태에서는 외부 페이지가LOGIN_COMPLETE/TOKEN_REFRESH/LOGOUT은 물론 이번에 추가된TIMER_*/NAVIGATE_BACK까지 호출할 수 있으니, 문자열 prefix 대신 파싱된origin을 정확히 비교해야 합니다.🔒 예시: 정확한 origin 비교로 수정
const handleMessage = useCallback(async (event: WebViewMessageEvent) => { - const origin = event.nativeEvent.url; - if (!origin || !ALLOWED_ORIGINS.some((allowed) => origin.startsWith(allowed))) { + let messageOrigin: string | null = null; + try { + messageOrigin = new URL(event.nativeEvent.url).origin; + } catch { + return; + } + + if (!ALLOWED_ORIGINS.includes(messageOrigin)) { return; } try { const data: NativeBridgeMessage = JSON.parse(event.nativeEvent.data);As per coding guidelines,
app/webview/[path].tsx: postMessage 수신 시 origin 검증이 반드시 이루어지는지 확인하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/webview/`[path].tsx around lines 93 - 100, The origin check in handleMessage is using startsWith on event.nativeEvent.url which can be bypassed; instead parse the URL (e.g., new URL(event.nativeEvent.url)) and compare its .origin exactly against ALLOWED_ORIGINS (use equality, not prefix), handle potential URL parse errors and reject messages when origin is not an exact match before JSON.parse of event.nativeEvent.data or any other processing tied to NativeBridgeMessage/TIMER_*/NAVIGATE_BACK/LOGIN_COMPLETE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/timerDisplayMode.ts`:
- Around line 90-119: enableTimerDisplayMode and disableTimerDisplayMode can run
concurrently and race on the module state (isKeepAwakeActive, brightness), so
serialize their effects: introduce a single shared operation mutex/queue (e.g.,
a pendingOperation Promise or simple mutex) used by both enableTimerDisplayMode
and disableTimerDisplayMode to await any in-flight operation before proceeding,
perform checks and calls to activateKeepAwakeAsync/deactivateKeepAwake and
dimScreenIfNeeded/restoreScreenBrightness while holding the lock, and only
update isKeepAwakeActive inside the serialized section after the corresponding
async call succeeds (ensure finally blocks also run under the lock or schedule
state changes through the mutex) so that overlapping calls cannot leave the
module in the wrong state; reference functions/variables to modify:
enableTimerDisplayMode, disableTimerDisplayMode, isKeepAwakeActive,
activateKeepAwakeAsync, deactivateKeepAwake, dimScreenIfNeeded,
restoreScreenBrightness, TIMER_KEEP_AWAKE_TAG.
---
Outside diff comments:
In `@app/webview/`[path].tsx:
- Around line 93-100: The origin check in handleMessage is using startsWith on
event.nativeEvent.url which can be bypassed; instead parse the URL (e.g., new
URL(event.nativeEvent.url)) and compare its .origin exactly against
ALLOWED_ORIGINS (use equality, not prefix), handle potential URL parse errors
and reject messages when origin is not an exact match before JSON.parse of
event.nativeEvent.data or any other processing tied to
NativeBridgeMessage/TIMER_*/NAVIGATE_BACK/LOGIN_COMPLETE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 663c7a94-105a-4915-9f6c-c7bf1fa19a6f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
app/webview/[path].tsxpackage.jsonservices/timerDisplayMode.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/timerDisplayMode.ts`:
- Around line 75-81: The finally block always setting isKeepAwakeActive = false
causes state to be out-of-sync when deactivateKeepAwake(TIMER_KEEP_AWAKE_TAG)
throws; change this so the local flag is only updated on successful
deactivation: remove the assignment from the finally, await deactivateKeepAwake
inside try and set isKeepAwakeActive = false in the successful path (after
await) and keep the catch to log the error; follow the same pattern as
activateKeepAwakeIfNeeded() so retries are possible when deactivate fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5bf7c79d-cc5a-4704-91bf-65c139cc2771
📒 Files selected for processing (1)
services/timerDisplayMode.ts
배경
변경 사항
expo-brightness,expo-keep-awake를 추가하고 관련 Expo 패키지 버전을 정리했습니다.TIMER_ACTIVE,TIMER_INACTIVE,NAVIGATE_BACK처리를 추가했습니다.검증
pnpm lintpnpm test -- --watchAll=false(No tests found, exiting with code 0)이슈
close #61
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정 / 개선
기타