Skip to content

Add support for JSConfirm dialog in FrontendScreen#6749

Merged
TimoPtr merged 5 commits intomainfrom
feature/js_confirm_dialog
Apr 29, 2026
Merged

Add support for JSConfirm dialog in FrontendScreen#6749
TimoPtr merged 5 commits intomainfrom
feature/js_confirm_dialog

Conversation

@TimoPtr
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr commented Apr 22, 2026

Summary

This PR introduced the first dialog of the FrontendScreen. I made a first iteration of the architecture but it is most probably going to change further we add dialogs to fit the requirements.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Screenshots

image image

Any other notes

I used this code in a iFrame to test

<!DOCTYPE html>
<html>
<body style="font-family: sans-serif; padding: 20px;">
<h2>JS Confirm Dialog Test</h2>
<button onclick="testConfirm()">Test confirm()</button>
<p id="result"></p>
<script>
    function testConfirm() {
      var result = confirm("Are you sure you want to proceed?");
      document.getElementById("result").textContent =
        "User chose: " + (result ? "OK" : "Cancel");
    }
</script>
</body>
</html>

@TimoPtr TimoPtr added the WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav. label Apr 22, 2026
@TimoPtr TimoPtr requested a review from jpelgrom April 22, 2026 13:26
@TimoPtr TimoPtr marked this pull request as ready for review April 22, 2026 13:26
Copilot AI review requested due to automatic review settings April 22, 2026 13:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds initial support for handling JavaScript confirm() dialogs in the Compose-based FrontendScreen flow, wiring WebView callbacks through the FrontendViewModel into a Compose dialog surface and updating theme tokens to match Material3 dialog styling.

Changes:

  • Extend HAWebChromeClient and FrontendViewModel to intercept onJsConfirm and surface it as pendingDialog in FrontendViewState.Content
  • Introduce a small dialog model/renderer (FrontendDialog, PendingDialogHandler, SimpleConfirmDialog) and render it from FrontendScreenContent
  • Add unit/UI tests plus screenshot test coverage for the new dialog UI state

Reviewed changes

Copilot reviewed 11 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
common/src/main/kotlin/io/homeassistant/companion/android/common/compose/theme/HATheme.kt Sets surfaceContainerHigh to ensure Material3 AlertDialog container color matches HA surface
app/src/main/kotlin/io/homeassistant/companion/android/util/HAWebChromeClient.kt Adds onJsConfirm callback plumbing from WebView into app code
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt Creates FrontendDialog.Confirm from JS confirms and stores it in view state
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewState.kt Adds pendingDialog field to the Content view state
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendScreen.kt Renders pending dialogs via PendingDialogHandler in the Compose screen
app/src/main/kotlin/io/homeassistant/companion/android/frontend/dialog/FrontendDialog.kt Defines dialog model for frontend-triggered dialogs
app/src/main/kotlin/io/homeassistant/companion/android/frontend/dialog/PendingDialogHandler.kt Routes dialog model to a concrete dialog composable
app/src/main/kotlin/io/homeassistant/companion/android/frontend/dialog/SimpleConfirmDialog.kt Implements the confirm dialog UI using AlertDialog + HA buttons/styles
app/src/test/kotlin/io/homeassistant/companion/android/util/HAWebChromeClientTest.kt Unit tests for new HAWebChromeClient.onJsConfirm behavior
app/src/test/kotlin/io/homeassistant/companion/android/frontend/dialog/SimpleConfirmDialogTest.kt Robolectric Compose UI tests for confirm dialog rendering and callbacks
app/src/screenshotTest/kotlin/io/homeassistant/companion/android/frontend/FrontendScreenScreenshotTest.kt Adds screenshot preview for FrontendScreenContent with confirm dialog
app/src/screenshotTestFullDebug/reference/...png New golden images for the confirm dialog screenshot tests

internal fun SimpleConfirmDialog(pendingDialog: FrontendDialog.Confirm) {
AlertDialog(
onDismissRequest = pendingDialog.onCancel,
title = { Text(text = stringResource(commonR.string.app_name), style = HATextStyle.Headline) },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the screenshots, the title looks too big compared to the content IMO. Compare with Chrome showing a dialog on Android which is also M3 themed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think HeadlineMedium is too big but I guess that's a limitation of the HA design system at this point
Screenshot showing confirm dialog title size in Chrome and Home Assistant

Copy link
Copy Markdown
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and confirmed working (although it previously already did dialogs from the view world, so I guess 'not broken' is more correct).

Comment on lines -70 to -96
/**
* Single-slot holder that queues permission requests.
*
* [emit] suspends until the slot is free (value is `null`), ensuring only one permission
* request is active at a time. [clear] frees the slot and unblocks the next waiting caller.
*/
@OptIn(ExperimentalForInheritanceCoroutinesApi::class)
private class PendingPermissionManager(
private val _state: MutableStateFlow<PermissionRequest<*>?> = MutableStateFlow(null),
) : StateFlow<PermissionRequest<*>?> by _state.asStateFlow() {
private val mutex = Mutex()
suspend fun emit(request: PermissionRequest<*>) {
mutex.withLock {
// Wait for the slot to be free, then fill it atomically.
// The mutex ensures only one caller can observe null and set a value,
// preventing concurrent emitters from overwriting each other.
_state.first { it == null }
_state.value = request
}
}

fun clear() {
_state.value = null
}
}

private val _pendingPermissionRequest = PendingPermissionManager()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice reuse!

@TimoPtr TimoPtr merged commit 9e4e381 into main Apr 29, 2026
55 of 59 checks passed
@TimoPtr TimoPtr deleted the feature/js_confirm_dialog branch April 29, 2026 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants