ADFA-3644 | Support pop-out tooltip display for tablets and DeX#1164
Merged
ADFA-3644 | Support pop-out tooltip display for tablets and DeX#1164
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 Walkthrough
Risks / Best-practice considerations:
High-level code changes:
Testing recommendations:
WalkthroughAdds a DeviceFormFactor utility and updates HelpActivity to support a companion launch(...) and runtime intent updates; ToolTipManager now calls HelpActivity.launch(...) instead of constructing Intents itself. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Tip as "ToolTipManager"
participant Help as "HelpActivity"
participant FF as "DeviceFormFactorUtils"
participant System as "Android System"
User->>Tip: click help link (url, title)
Tip->>Help: launch(context, url, title)
Help->>FF: getCurrent(context)
FF->>System: query uiMode / resources / metrics / Samsung services
System-->>FF: device metrics / flags
FF-->>Help: DeviceFormFactor(isTablet, isDexMode)
Help->>Help: build Intent (set data/extras, conditional flags)
Help->>System: startActivity(intent)
System->>Help: onCreate()/onNewIntent(intent)
Help->>Help: updateUIFromIntent(intent) -> set title, load WebView URL
Help-->>User: render help content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt (1)
92-145:⚠️ Potential issue | 🟠 MajorRemove the duplicate initial
loadUrl()call and update the title inonNewIntent().
onCreate()loadsCONTENT_KEYtwice: once at line 134 viahtmlContent?.let { webView.loadUrl(url) }, then again at line 144 vialoadUrlFromIntent(intent). This creates duplicate WebView history entries.Additionally,
onNewIntent()reloads content but does not refresh the action bar title when a newCONTENT_TITLE_KEYarrives, leaving stale titles on multi-window activity reuse.🧭 Suggested fix
val pageTitle = intent.getStringExtra(CONTENT_TITLE_KEY) - val htmlContent = intent.getStringExtra(CONTENT_KEY) supportActionBar?.title = pageTitle ?: getString(R.string.help) // Configure WebView settings for localhost access @@ -130,10 +128,6 @@ } } - // Load the HTML file from the assets folder - htmlContent?.let { url -> - webView.loadUrl(url) - } } // Set up back navigation callback for system back button @@ -147,6 +141,9 @@ override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) + val pageTitle = intent.getStringExtra(CONTENT_TITLE_KEY) + supportActionBar?.title = pageTitle ?: getString(R.string.help) loadUrlFromIntent(intent) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt` around lines 92 - 145, Remove the duplicate WebView navigation by only loading the URL via loadUrlFromIntent(intent) (avoid the htmlContent?.let { webView.loadUrl(url) } call in onCreate) so the CONTENT_KEY is not loaded twice into history; also update onNewIntent(intent) to call supportActionBar?.title = intent.getStringExtra(CONTENT_TITLE_KEY) (and then call loadUrlFromIntent(intent) / handle CONTENT_KEY) so the action bar title is refreshed when a new CONTENT_TITLE_KEY arrives. Use the existing helper loadUrlFromIntent and the onNewIntent override to centralize both title and URL updates.
🧹 Nitpick comments (1)
common/src/main/java/com/itsaky/androidide/utils/DeviceFormFactor.kt (1)
49-120: Don’t swallow every failure in form-factor detection.These helpers currently convert any unexpected error into
falseviacatch (_: Exception)andrunCatching. Because this result controls whether the adjacent-window flow is used, silent fallback here makes platform regressions look like normal phone behavior instead of surfacing them.🔧 Suggested direction
Catch only the reflective/lookup failures you expect here, e.g.
ReflectiveOperationException,SecurityException, andClassCastException, and replacerunCatchinginreadSamsungDesktopModeValue()with an explicit fallback so unexpected failures still fail fast during development.Based on learnings, Kotlin code in this repo should catch only the specific exception types expected from the runtime failure being handled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/utils/DeviceFormFactor.kt` around lines 49 - 120, The helpers (isPhysicalLargeScreen, isSamsungDexModeModern, isSamsungDexModeLegacy and readSamsungDesktopModeValue) currently swallow all Exceptions; change them to only catch the expected reflection/lookup/security errors (for example ReflectiveOperationException, SecurityException, ClassCastException) and let other exceptions propagate. Replace the blanket try/catch in isPhysicalLargeScreen and isSamsungDexModeModern with try blocks that catch those specific exception types and return false only for them; update readSamsungDesktopModeValue to remove runCatching/recoverCatching and instead perform the explicit getField(...) then fallback to getDeclaredField(...) inside targeted catch clauses that return null only for the expected reflection/security exceptions. Ensure you reference and modify the functions named isPhysicalLargeScreen, isSamsungDexModeModern, isSamsungDexModeLegacy, and readSamsungDesktopModeValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt`:
- Around line 147-156: onNewIntent currently calls loadUrlFromIntent but never
updates the action-bar title; modify loadUrlFromIntent (or its call-site) to
also read the title extra from the intent (e.g.
intent.getStringExtra(TITLE_KEY)) and update the UI by calling
supportActionBar?.title = title (or setTitle(title) / binding.toolbar.title =
title) so the action-bar reflects the newly loaded content when onNewIntent is
used.
---
Outside diff comments:
In
`@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt`:
- Around line 92-145: Remove the duplicate WebView navigation by only loading
the URL via loadUrlFromIntent(intent) (avoid the htmlContent?.let {
webView.loadUrl(url) } call in onCreate) so the CONTENT_KEY is not loaded twice
into history; also update onNewIntent(intent) to call supportActionBar?.title =
intent.getStringExtra(CONTENT_TITLE_KEY) (and then call
loadUrlFromIntent(intent) / handle CONTENT_KEY) so the action bar title is
refreshed when a new CONTENT_TITLE_KEY arrives. Use the existing helper
loadUrlFromIntent and the onNewIntent override to centralize both title and URL
updates.
---
Nitpick comments:
In `@common/src/main/java/com/itsaky/androidide/utils/DeviceFormFactor.kt`:
- Around line 49-120: The helpers (isPhysicalLargeScreen,
isSamsungDexModeModern, isSamsungDexModeLegacy and readSamsungDesktopModeValue)
currently swallow all Exceptions; change them to only catch the expected
reflection/lookup/security errors (for example ReflectiveOperationException,
SecurityException, ClassCastException) and let other exceptions propagate.
Replace the blanket try/catch in isPhysicalLargeScreen and
isSamsungDexModeModern with try blocks that catch those specific exception types
and return false only for them; update readSamsungDesktopModeValue to remove
runCatching/recoverCatching and instead perform the explicit getField(...) then
fallback to getDeclaredField(...) inside targeted catch clauses that return null
only for the expected reflection/security exceptions. Ensure you reference and
modify the functions named isPhysicalLargeScreen, isSamsungDexModeModern,
isSamsungDexModeLegacy, and readSamsungDesktopModeValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a7a6e33-dc5d-4c18-b36f-eecb0419273f
📒 Files selected for processing (3)
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.ktcommon/src/main/java/com/itsaky/androidide/utils/DeviceFormFactor.ktidetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt
Show resolved
Hide resolved
Collaborator
Author
|
Current behavior document_5093759534346798867.mp4 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt (1)
92-95:⚠️ Potential issue | 🟡 MinorRemove duplicate title/URL handling to avoid loading content twice.
The new
updateUIFromIntent(intent)call at line 144 duplicates work already done earlier inonCreate:
- Title is set at line 95, then again in
updateUIFromIntent(line 155)webView.loadUrl(url)is called at line 134, then again inupdateUIFromIntent(line 158)The double
loadUrl()call will trigger two network requests for the same content.Proposed fix: remove duplicate logic and rely on updateUIFromIntent
with(binding) { setSupportActionBar(toolbar) supportActionBar!!.setDisplayHomeAsUpEnabled(true) toolbar.setNavigationOnClickListener { handleBackNavigation() } // Set status bar icons to be dark in light mode and light in dark mode WindowCompat.getInsetsController(this@HelpActivity.window, this@HelpActivity.window.decorView).apply { isAppearanceLightStatusBars = !isSystemInDarkMode() isAppearanceLightNavigationBars = !isSystemInDarkMode() } - val pageTitle = intent.getStringExtra(CONTENT_TITLE_KEY) - val htmlContent = intent.getStringExtra(CONTENT_KEY) - - supportActionBar?.title = pageTitle ?: getString(R.string.help) // Configure WebView settings for localhost access webView.settings.javaScriptEnabled = true webView.settings.allowFileAccess = true webView.settings.allowFileAccessFromFileURLs = true webView.settings.allowUniversalAccessFromFileURLs = true webView.settings.domStorageEnabled = true webView.settings.databaseEnabled = true webView.settings.mixedContentMode = android.webkit.WebSettings.MIXED_CONTENT_ALWAYS_ALLOW // Set WebViewClient to handle page navigation within the WebView webView.webViewClient = object : WebViewClient() { // ... unchanged ... } - - // Load the HTML file from the assets folder - htmlContent?.let { url -> - webView.loadUrl(url) - } } // Set up back navigation callback for system back button onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) { override fun handleOnBackPressed() { handleBackNavigation() } }) updateUIFromIntent(intent) }Also applies to: 132-135, 144-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt` around lines 92 - 95, In onCreate, remove the redundant title and initial load code so updateUIFromIntent(intent) is the single source of truth: delete the retrieval/assignment of pageTitle/htmlContent, the supportActionBar?.title = ... call and the webView.loadUrl(url) that occur before updateUIFromIntent(intent); leave updateUIFromIntent(intent) to set supportActionBar?.title and call webView.loadUrl(url) (ensure updateUIFromIntent handles null/defaults), so updateUIFromIntent(intent) is the only place that sets the ActionBar title and loads the WebView URL.
🧹 Nitpick comments (1)
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt (1)
44-64: Consider documenting or enforcing Activity context requirement for accurate form factor detection.The
launch()method accepts a genericContext, butDeviceFormFactorUtils.getCurrent(context)relies onWindowManagermetrics that may be inaccurate when passed an Application context instead of an Activity context (per the implementation inDeviceFormFactor.ktwhich useswindowManager.maximumWindowMetrics).If an Application context is passed, large-screen detection could be incorrect, causing the wrong launch flags to be applied. Consider either:
- Adding a KDoc comment noting the requirement for Activity context for accurate detection
- Using a method signature that takes
Activitywhen called from tooltip contexts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt` around lines 44 - 64, The HelpActivity.launch method currently accepts a generic Context but uses DeviceFormFactorUtils.getCurrent(context) which can return incorrect metrics for non-Activity contexts; update launch to either require an Activity parameter (change signature to launch(activity: Activity, url: String, title: String) and use that activity when calling DeviceFormFactorUtils.getCurrent) or, if you must keep Context, add a KDoc to launch documenting that an Activity Context is required for correct form-factor detection and add a runtime guard that treats non-Activity contexts as non-large (e.g., if (context is Activity) use getCurrent(context) else assume small screen) so flags are not wrongly applied; reference HelpActivity.launch and DeviceFormFactorUtils.getCurrent when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt`:
- Around line 92-95: In onCreate, remove the redundant title and initial load
code so updateUIFromIntent(intent) is the single source of truth: delete the
retrieval/assignment of pageTitle/htmlContent, the supportActionBar?.title = ...
call and the webView.loadUrl(url) that occur before updateUIFromIntent(intent);
leave updateUIFromIntent(intent) to set supportActionBar?.title and call
webView.loadUrl(url) (ensure updateUIFromIntent handles null/defaults), so
updateUIFromIntent(intent) is the only place that sets the ActionBar title and
loads the WebView URL.
---
Nitpick comments:
In
`@common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt`:
- Around line 44-64: The HelpActivity.launch method currently accepts a generic
Context but uses DeviceFormFactorUtils.getCurrent(context) which can return
incorrect metrics for non-Activity contexts; update launch to either require an
Activity parameter (change signature to launch(activity: Activity, url: String,
title: String) and use that activity when calling
DeviceFormFactorUtils.getCurrent) or, if you must keep Context, add a KDoc to
launch documenting that an Activity Context is required for correct form-factor
detection and add a runtime guard that treats non-Activity contexts as non-large
(e.g., if (context is Activity) use getCurrent(context) else assume small
screen) so flags are not wrongly applied; reference HelpActivity.launch and
DeviceFormFactorUtils.getCurrent when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eeb2fdd1-b3a2-4a21-b187-e39d01fd71e8
📒 Files selected for processing (1)
common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa,
itsaky-adfa and
jomen-adfa
jomen-adfa
approved these changes
Apr 8, 2026
Refactors HelpActivity launch logic and adds device form factor detection
jatezzz
force-pushed
the
feat/ADFA-3644-help-activity-multi-window
branch
from
4bfca2f to
7b6f5fe
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refactored
ToolTipManagerandHelpActivityto support opening tooltip help in a separate, adjacent window when the user is on a tablet or Samsung DeX device.DeviceFormFactorUtilsto reliably detect large screens and DeX modes, utilizing physical screen metrics and Samsung's hidden APIs to maintain accurate detection even if the primary app window is resized.HelpActivity.SINGLE_TOPwith a static URI to recycle the secondary window, preventing the creation of infinite windows when users click multiple tooltips sequentially.Details
document_5093759534346798852.mp4
Ticket
ADFA-3644
Observation
The
DeviceFormFactorUtilsusesmaximumWindowMetrics(orgetRealMetricson legacy) and reflection for Samsung DeX to guarantee the application doesn't incorrectly fall back to the single-window mobile layout when the user shrinks the IDE window in a desktop environment.