fix: make next and previous action work for Immich Kiosk (WebView)

[robinsmith-source]

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]

📝 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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]

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))

[robinsmith-source]

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]

not sure, works as expected with our app.

[3rob3]

Can you add this to pauseAction also?
dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SPACE))
dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_SPACE))

[robinsmith-source]

@3rob3 To make things clear:
Originally I've created this PR, because I wanted to make the Remote Control Server also work for the Immich Kiosk Application when displaying it as a Webpage inside Immich Frame. Currently next, previous and pause aren't working for Immich Kiosk. With the Addition of the ACTION_UP it initially solved it for (next and previous), because Immich Kiosk is/was only looking for Keydown Events here and therefore Keyup Events got omitted and never processed.

The Pause didn't work because of the Keycode " " instead of the expected "Space" was sent by the Kiosk Browser in Immich Frame. I'm not that deep in the Android/Kotlin Ecosystem so I only could assume that this is a conversion/mapping Issue between Kotlin/Android Native Keycodes and Keycodes in Javascript.

Therefore I think it's better to adjust those things directly in the ImmichKiosk project. Please let me know if you have some other thoughts about this.

Nevertheless I still can add the requested changes, without the ACTION_UP line as it's probably not needed anymore.

[3rob3]

I'm confused.....so close this PR then? Why is ACTION_UP no longer needed?