-
Notifications
You must be signed in to change notification settings - Fork 11
fix: make next and previous action work for Immich Kiosk (WebView) #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: make next and previous action work for Immich Kiosk (WebView) #43
Conversation
📝 WalkthroughWalkthroughThe DPAD navigation key event dispatching in MainActivity.kt was modified to send separate ACTION_DOWN and ACTION_UP events sequentially instead of dispatching a single KeyEvent object, altering the key event sequence timing for WebView navigation simulation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
685-700: Apply the same fix topauseAction()to make pause work for Immich Kiosk.You mentioned pause doesn't work for Immich Kiosk. The issue is that
pauseAction()only dispatchesACTION_DOWNwhile your fix forpreviousAction()andnextAction()dispatches bothACTION_DOWNandACTION_UP. Apply the same pattern here.🛠️ Proposed fix
private fun pauseAction() { if (useWebView) { // Simulate a key press webView.requestFocus() - val event = KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SPACE) - dispatchKeyEvent(event) + dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SPACE)) + dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_SPACE)) } else { zoomAnimator?.cancel() if (isImageTimerRunning) {
🧹 Nitpick comments (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
656-683: Consider applying the same fix toScreenSaverService.kt.The
previousAction()andnextAction()methods inScreenSaverService.ktstill use the old pattern (onlyACTION_DOWNviawebView.dispatchKeyEvent()). If the screensaver service also supports Immich Kiosk WebView mode, you may want to apply the same fix there for consistency.
|
looks fine to me. Regarding Pause, probably need top add it there as well (in pauseAction)? |
Yes code-style wise it should probably also applied there. But like I've described in the PR I haven't got it to work with that. |
|
not sure, works as expected with our app. |
This PR fixes the
nextandpreviousactions from the remote control api to also work for Immich Kiosk.Unfortunately I haven't managed to get
pauseto work on Immich Kiosk as well. Would appreciate some help or further advise here!Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.