Repository navigation
fix(web): detect SPA navigation while find-in-page is active - #398
Conversation
🐤 Kanarek · 🟣 merged
App · 1 files |
Reviewer's GuideThe Find overlay now detects SPA pushState/replaceState navigation by polling the active WebView URL and location.href every 300 ms only during the active Find session, closing promptly on URL changes while preventing stale callbacks and data retention. Sequence diagram for Find overlay SPA navigation detectionsequenceDiagram
participant User
participant FindOverlay
participant WebChatScreen
participant WebView
User->>FindOverlay: Open Find
FindOverlay->>WebChatScreen: Start LaunchedEffect(findSession)
loop Every 300 ms while Find is active
WebChatScreen->>WebView: Read webView.url
alt URL differs from activeSearch.pageUrl
WebChatScreen->>FindOverlay: closeFindInPage()
else URL unchanged
WebChatScreen->>WebView: evaluateJavascript(window.location.href)
WebView-->>WebChatScreen: location.href
alt href differs from activeSearch.pageUrl
WebChatScreen->>FindOverlay: closeFindInPage()
end
end
end
User->>FindOverlay: Close Find
FindOverlay-->>WebChatScreen: Clear findSession
WebChatScreen-->>WebChatScreen: Stop polling
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt" line_range="188-189" />
<code_context>
+ while (findSession === activeSearch) {
+ delay(300L)
+ val webView = activeSearch.webView
+ if (webView.url?.let { it != activeSearch.pageUrl } == true) {
+ closeFindInPage()
+ break
+ }
</code_context>
<issue_to_address>
**Find closes before navigation**
When find opens before `webView.url` is assigned, `activeSearch.pageUrl` is null, so the `webView.url` check treats the first assigned URL as navigation, and the `evaluateJavascript` callback compares against `""`; either path calls `closeFindInPage()` and dismisses Find before the user can search the loaded page.
Only treat a URL as changed when `activeSearch.pageUrl` is non-null, in both checks.
Also at `app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt:194-195`.
</issue_to_address>
### Comment 2
<location path="app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt" line_range="183-185" />
<code_context>
}
+ // pushState/replaceState in a SPA do not consistently trigger WebViewClient callbacks.
+ // Watch only while Find is visible; never retain or log the provider URL.
+ LaunchedEffect(findSession) {
+ val activeSearch = findSession ?: return@LaunchedEffect
+ while (findSession === activeSearch) {
+ delay(300L)
+ val webView = activeSearch.webView
</code_context>
<issue_to_address>
**Hidden tab keeps polling**
When the user switches tabs while Find is open, `LaunchedEffect(findSession)` does not check `isActive`, so the loop keeps polling the hidden WebView every 300 ms; a URL change also calls `closeFindInPage()` and loses the retained Find session when the user returns.
Gate the polling effect on `isActive` so it stops while the tab is inactive.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt:189, app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt:185
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟠 High · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: QUIET
- Plan: Advanced
- Run ID:
9bf3be4f-d5d4-4cf5-a9ac-66bd38516aac
📒 Files selected for processing: 1
app/src/main/java/ais/tee/ui/screens/WebChatScreen.kt
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Fix
location.hrefat a bounded 300 ms interval.history.pushStateorreplaceStatewithout triggeringWebViewClientnavigation callbacks.Why
The unchanged Android instrumentation test
WebChatFindTest.sameDocumentConversationNavigationClosesFindtimed out in both Aistee Claude PRs #396 and #397 (71/72 passing). The existingonUrlChangedcallback does not reliably fire for same-document navigation on Android 16 WebView.CI including Android smoke is the validation gate. Profile regeneration unverified; this affects the Find overlay only, not benchmarked startup/provider-switch journeys.