Skip to content

Conversation

@robinsmith-source
Copy link

@robinsmith-source robinsmith-source commented Jan 26, 2026

This PR fixes the next and previous actions from the remote control api to also work for Immich Kiosk.

Unfortunately I haven't managed to get pause to work on Immich Kiosk as well. Would appreciate some help or further advise here!

Summary by CodeRabbit

  • Bug Fixes
    • Improved DPAD left and right navigation key event handling to ensure consistent behavior when navigating within the app interface.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

The 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

Cohort / File(s) Summary
Key Event Dispatching
app/src/main/java/com/immichframe/immichframe/MainActivity.kt
Modified previousAction and nextAction methods to dispatch DPAD left/right navigation as paired ACTION_DOWN followed by ACTION_UP events instead of single KeyEvent objects

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A pair of taps, down then up,
DPAD keys dance in rhythmic cup,
Navigation flows with new intent,
Sequential events heaven-sent!
Left and right, the journey's long. 🎮

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing next and previous remote actions for Immich Kiosk WebView.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 to pauseAction() to make pause work for Immich Kiosk.

You mentioned pause doesn't work for Immich Kiosk. The issue is that pauseAction() only dispatches ACTION_DOWN while your fix for previousAction() and nextAction() dispatches both ACTION_DOWN and ACTION_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 to ScreenSaverService.kt.

The previousAction() and nextAction() methods in ScreenSaverService.kt still use the old pattern (only ACTION_DOWN via webView.dispatchKeyEvent()). If the screensaver service also supports Immich Kiosk WebView mode, you may want to apply the same fix there for consistency.

@3rob3
Copy link
Contributor

3rob3 commented Jan 27, 2026

looks fine to me. Regarding Pause, probably need top add it there as well (in pauseAction)?

dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SPACE))
dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_SPACE))

@3rob3 3rob3 added the bug Something isn't working label Jan 27, 2026
@robinsmith-source
Copy link
Author

looks fine to me. Regarding Pause, probably need top add it there as well (in pauseAction)?

dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SPACE))
dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_SPACE))

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.

@3rob3
Copy link
Contributor

3rob3 commented Jan 27, 2026

not sure, works as expected with our app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants