Delete the fullscreen Input Screen implementation - #9392
Merged
malmstein merged 4 commits intoAug 6, 2026
Merged
Conversation
Contributor
|
The suggestions list, chat-suggestion readers, metrics, the onboarding wide event, the base input widget and the config resolver are all used by the native input widget, so they move out before the Input Screen is deleted. Package renames only, no behaviour change. The app-module lint baseline entries are rewritten to the new import paths: the underlying NoImplImportsInAppModule violations are unchanged, only the FQN moved. InputScreenActivity and InputScreenFragment gain an import for the moved config resolver; both are deleted in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removes the activity, fragment, tab fragments, view models, state and command types, tab attachments, the session store and the input-screen only views, plus the manifest entry, layouts, drawable, dead dimens and strings, and the .maestro/input_screen suite with its inputScreenTest tag. InputScreenConfigResolver goes too: every member it exposed read intent extras that only InputScreenActivity ever set, so on the native path all five were constants. The two native call sites now use those constants directly (no installed apps in autocomplete, top-bar omnibar type). InputScreenButtons loses its default layout, both native callers pass one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The launch params, result codes, result params, browser-buttons config and the transition provider all lost their last consumer when the Input Screen went away. DuckAiOnboardingEndCtaVariant is still used by CtaViewModel, so it moves to its own file in the same package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DUCK_CHAT_OPEN_AUTOCOMPLETE_EXPERIMENTAL had no fire site left. Every other candidate on the cleanup list is still fired from the native input widget, directly or through RealDuckChatPixels, so those stay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
malmstein
force-pushed
the
feature/david/delete_input_screen_impl
branch
from
August 4, 2026 17:01
6002145 to
6f8f3b8
Compare
6 tasks
7 tasks
| import kotlinx.coroutines.sync.withLock | ||
| import javax.inject.Inject | ||
|
|
||
| interface InputScreenDiscoveryFunnel { |
Collaborator
There was a problem hiding this comment.
Nit: Any reason not to rename the interface/class too?
YoussefKeyrouz
approved these changes
Aug 6, 2026
YoussefKeyrouz
left a comment
YoussefKeyrouz
left a comment
Collaborator
There was a problem hiding this comment.
Looks good. Tested most scenarios I can think about. Code wise it’s majority delete/moving things around and rebaselining. Thanks David, great cleanup.
malmstein
added a commit
that referenced
this pull request
Aug 6, 2026
Task/Issue URL: https://app.asana.com/1/137249556945/task/1217152561538192 Tech Design URL (if applicable): API Proposals URL(s) (if applicable): None ### Description **Stacked on #9392 (which is itself stacked on #9391) — review and merge those first. This PR targets #9392's branch, so the diff here is only the third step.** Last PR of the Input Screen removal series. PR 2 deleted the fullscreen Input Screen, which left `InputModeWidget` with exactly one subtype. This collapses the base class into it. Structural merge, no behaviour change. The class keeps the name `NativeInputModeWidget`: three XML layouts reference the FQN and the inflater needs the current package, name and `@JvmOverloads (context, attrs, defStyle)` constructor, so a rename buys clarity nowhere it is visible. **Merge (commit 1).** `NativeInputModeWidget` now extends `ConstraintLayout` directly and takes `@HasMemberInjections`, since it owns member injection. The base's live members move in: `pixel` and `duckChatInternal`, the view bindings, `configureClickListeners`/`configureInputBehavior`/`configureTabBehavior`/`configureShadow`, the mode-selection tab listener with its attach/detach wiring, the host-facing callbacks, `text`, `submitMessage` (base body inlined into the override where the `super` call was), `submitAsChat`, `printNewLine`, `selectAllText`, `clearInputFocus`, `isChatTabSelected`, `setMainButtonsVisible` and the `fade` helper. `onAttachedToWindow`/`onDetachedFromWindow` preserve the original statement order across the former class boundary, including `AndroidSupportInjection.inject(this)` running before `super`. Members that only ever served the fullscreen Input Screen are dropped rather than ported: the menu and tab-switcher taps, tab attachments and their tag span, the expand/collapse transition machinery, `provideInitialInputState`/`initOnSearch`/`initOnChat`, `getSelectedTabPosition`, `selectTab`, `setScrollPosition`, `setInputScreenBottomButtons`/`setInputScreenButtonsVisible`, `setMenuIcon`, `setBrowserMenuHighlightVisible`, `getAnchorView` and `cardContext`. Each was verified to have zero references outside the deleted code. Simplifications the merge enables, all behaviour-preserving: * `canExpand` and `bottomButtonsMode` were always `false` on the native path, so `applyModeSpecificInputBehaviour` inlines to the values it actually produced (search: `maxLines = 1`; chat: `minLines`/`maxLines` = 2, horizontal scrolling then overridden by `applyChatInputType`). * The clear-button and editor-action listeners the base installed in `init` were replaced at attach by the hooked versions that also fire pixels, so only the hooked ones remain. * The scattered re-`findViewById` calls now use the view fields bound once in `init`. **Delete (commit 2).** `InputModeWidget.kt`, `TabAttachmentTagSpan.kt`, `view_input_mode_switch_widget.xml`, two stale lint-baseline entries, and the dangling `[InputModeWidget]` KDoc links in `RealDuckChat`. Three notes where reality departed from the plan: * **The pixel-retirement step turned out to be a no-op.** The plan expected `DUCK_CHAT_EXPERIMENTAL_OMNIBAR_KEYBOARD_GO_PRESSED` to die with the base's editor-action listener, but the surviving hooked listener fires the same pixel through `viewModel.fireKeyboardGoPressed()` → `RealDuckChatPixels.fireOmnibarKeyboardGoPressed()`. Same for `..._CLEAR_BUTTON_PRESSED` via `fireClearPressed`. `..._BACK_BUTTON_PRESSED` fires from the merged `onBackPressed()`. All three stay; there is no third commit. * **`attachmentsContainer` needed a home.** The deleted layout was that id's declaration site (`@+id/`), and the native layout references it while `AttachmentView` looks it up — resource linking caught this. It moves to `native_input_plugin_ids.xml` next to the other plugin container ids. * **Two internal `submitMessage()` calls are now explicit** (`submitMessage(message = null)`): the default argument lived on the base method, and Kotlin does not allow an override to declare one. The external caller already passed a query. Dropping the base's no-op click listeners on the widget's own browser-menu and tab-switcher views is safe: both callbacks were never set, and both views live inside `inputModeMainButtonsContainer`, which both native hosts hide via `hideMainButtons()`. `addTabClickListeners` goes for the same reason — it only fired the dead `onTabTapped`, and `TabLayout` selects a tab on tap by default. ### Steps to test this PR Verified on a Pixel 9 (Android 16) internal build: widget attaches with no member-injection failure, text entry drives the clear button and suggestions, the Duck.ai tab renders the bottom row (attachments, options, reasoning, model picker, send) and the floating new-line button, the Search tab collapses to a single line with search suggestions and no bottom row, and the back arrow renders. No crashes or `lateinit` errors throughout. _Native widget in the browser_ - [x] Focus, type, submit a search, submit a chat - [x] Clear button, voice button, fire button, tab switcher count, back button - [x] Top, bottom and split omnibar _Contextual Duck.ai sheet_ (`fragment_contextual_duck_ai_native.xml` inflates the same class) - [x] Open, type, submit _Injection lifecycle_ (the highest-risk change: `pixel`/`duckChatInternal` now inject into the merged class) - [x] Rotate the device, the attach/detach cycle re-runs injection with no crash - [x] Background and restore the app ### UI changes None. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Large structural change to the unified search/chat input widget and DI attach/detach order, but intended as behavior-preserving; regression risk is concentrated in omnibar, contextual sheet, and rotation/background lifecycle. > > **Overview** > **Collapses the obsolete `InputModeWidget` base class into `NativeInputModeWidget`**, which now extends `ConstraintLayout` directly and owns member injection (`pixel`, `duckChatInternal`). Shared omnibar behavior—tab listeners syncing `DuckChatInputModeState`, input field wiring, `submitMessage`, attach/detach lifecycle—is inlined; repeated `findViewById` calls use fields bound in `init`. > > **Removes code that only served the deleted fullscreen Input Screen:** `InputModeWidget.kt`, `TabAttachmentTagSpan.kt`, `view_input_mode_switch_widget.xml`, tab-attachment tagging, browser menu/tab-switcher handlers on the widget, and expand/collapse transition paths (`canExpand` / `bottomButtonsMode` inlined to fixed line counts on the native path). > > **Housekeeping:** `RealDuckChat` KDoc links point at `NativeInputModeWidget`; stale lint baseline entries for the old layout are dropped; `attachmentsContainer` is declared in `native_input_plugin_ids.xml` so the native layout still links. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a51821d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
malmstein
added a commit
that referenced
this pull request
Aug 7, 2026
…9402) Task/Issue URL: https://app.asana.com/1/137249556945/project/1174433894299346/task/1217189225202021 Tech Design URL (if applicable): API Proposals URL(s) (if applicable): None ### Description **Stacked on #9394 (collapse InputModeWidget), which is stacked on #9392, which is stacked on #9391. Review and merge those first — this PR targets #9394's branch, so the diff here is only the fourth step.** Deletes the overlay-based fullscreen Duck.ai experience (`DuckChatWebViewFragment`/`DuckChatWebViewViewModel`, a fragment that slid in over the browser from the right) and the `showFullScreenMode` rollout flag that gated it. The flag's replacement — tab-based Duck.ai, opened via `BrowserActivity.launchDuckAi()`'s new-tab path — is already the only reachable behaviour once this lands; the flag defaulted `INTERNAL` and every non-internal caller already took that branch by the time this task was scoped, so there's no user-visible change. Four commits: 1. **Delete `DuckChatWebViewFragment`/`DuckChatWebViewViewModel`** and their layout/test. Nothing routes to them any more. 2. **Remove the flag and its infrastructure** — `showFullScreenMode`/`showFullScreenModeToggle` off `DuckAiFeatureState`, `DuckChatFeature.fullscreenMode()`, the `DUCK_CHAT_FULLSCREEN_MODE_SETTING` preference and its accessors. `isDuckChatFullScreenModeEnabled()` is a distinct, still-live accessor (feeds `DuckChatJSHelper`'s JS capability signalling) — it now tracks Duck.ai feature availability directly rather than the retired rollout computation, since that's the only gate left with any meaning once the fragment alternative is gone. 3. **Collapse every `showFullScreenMode.value` branch in `:app`** to its `true` side: `BrowserActivity.launchDuckAi()`/`CLOSE_DUCK_CHAT` handling, `BrowserViewModel.openDuckChat()`/`Command.OpenDuckChat`, `BrowserTabViewModel.evaluateDuckAIPage()`/`openDuckAiQuery()`/`openDuckAiChatById()`/`onDuckChatOmnibarButtonClicked()`, `TabSwitcherViewModel.onDuckAIButtonClicked()`, and `SpecialUrlDetector`'s `ShouldLaunchDuckChatLink` special-casing (only reachable when the flag was off). `ViewTransitionAnimations.kt` (bundled into commit 1) existed solely for the overlay's slide-in/out, so it goes with the last caller. 4. **Regenerate the app lint baseline** (`./gradlew :app:updateLintBaseline`) to drop the stale `NoImplImportsInAppModule` entries for the deleted import and reconcile line numbers. ### Deviations from the original task notes worth flagging in review - `isDuckChatFullScreenModeEnabled()`'s new definition (`isDuckChatFeatureEnabled` instead of the rollout-flag computation) wasn't in the task's listed steps — it fell out of removing `fullscreenMode()`, since this accessor is a separate, still-consumed concept from the `showFullScreenMode` StateFlow the task named. - The task's Step 5 said "two menu click handlers" checked the flag in `BrowserTabFragment`; only one still did (the other already called `openNewDuckChat` unconditionally) — the codebase moved since the task was scoped. - One pre-existing, previously-untested quirk surfaced once the `true` branch became unconditional: `BrowserTabViewModel.openDuckAiQuery()` fires `onInputSubmitted()` on browser-interaction plugins twice (once directly, once via the `onUserSubmittedQuery()` call it routes through). This predates the flag removal — it was already true whenever `showFullScreenMode` was on — so it's documented on the test rather than fixed here, to keep this PR a pure flag removal. - A prior attempt at this exact task ([#8134](#8134), linked from the Asana task's "First attempt" subtask) hardcoded `showNewDuckChatTabOption = false`, breaking the live new-Duck.ai-tab feature instead of just the dead overlay — a mix-up between the fragment's own `showFullScreenMode`-gated branches and the unrelated, still-live `hasSessionActive` concept threaded through `BrowserNav.openDuckChat()`. This PR verified each branch's *kept* side against production before collapsing it, and left `BrowserNav`/`AppBrowserNav`/`RealDuckChat`/the widget shortcut call sites untouched. ### Steps to test this PR - [x] Tap the omnibar Duck.ai button (unfocused, browser tab): opens a new Duck.ai tab - [x] Type in the omnibar, tap the Duck.ai chevron while focused: opens Duck.ai with the typed query pre-filled - [x] New tab page, tap "Ask Duck.ai" / submit a query with the Duck.ai toggle selected: opens in a new tab, NTP tab itself is reused (not duplicated) - [x] Tab switcher FAB → Duck.ai: opens a new Duck.ai tab and closes the switcher - [x] Navigate to a `duck.ai` URL directly (address bar or link): loads normally in the tab, no special-cased redirect - [x] Contextual sheet still opens from the omnibar when unfocused and not on NTP (unrelated `showContextualMode` path, untouched) - [x] `CLOSE_DUCK_CHAT` intent (e.g. widget/notification action) while in a Duck.ai tab: closes the tab ### UI changes None. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Broad Duck.ai navigation and URL-handling changes across `:app` and `duckchat`, but behavior is collapsed to the already-shipped tab path rather than new product logic. JS capability flags still gate on feature availability via the updated `isDuckChatFullScreenModeEnabled()` definition. > > **Overview** > Removes the **overlay fullscreen Duck.ai experience** (`DuckChatWebViewFragment` / `DuckChatWebViewViewModel`, its layout and tests, and `ViewTransitionAnimations` used only for that slide-in UI). Duck.ai entry points no longer branch on a retired rollout flag—they always use the **new-tab** path (e.g. `BrowserActivity.launchDuckAi()` opening a tab with a return tab id; `CLOSE_DUCK_CHAT` closes the current tab when it is Duck.ai). > > **Flag and preference cleanup:** `showFullScreenMode` / `showFullScreenModeToggle` on `DuckAiFeatureState`, `DuckChatFeature.fullscreenMode()`, and `DUCK_CHAT_FULLSCREEN_MODE_SETTING` are deleted. **`isDuckChatFullScreenModeEnabled()`** remains for JS capability signaling in `DuckChatJSHelper` and now mirrors **`isDuckChatFeatureEnabled()`** instead of the old rollout computation. > > **`:app` simplification:** `BrowserViewModel`, `BrowserTabViewModel`, `TabSwitcherViewModel`, and `SpecialUrlDetector` lose flag-gated branches (`Command.OpenDuckChat` overlay path, `ShouldLaunchDuckChatLink` special redirect when the flag was off, etc.). **`duck.ai` URLs load normally in the current tab.** App lint baseline is regenerated for deleted imports and line shifts. > > No intended UI change for users already on tab-based Duck.ai (the flag’s effective default). A **pre-existing** double `onInputSubmitted` via `openDuckAiQuery()` when the flag was on is noted in tests, not fixed here. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e17fdbb. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Task/Issue URL: https://app.asana.com/1/137249556945/task/1217152427162078
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable):
Description
Stacked on #9391 — review and merge that one first. This PR targets its branch, so the diff here is only the second step.
Second PR of the Input Screen removal series. PR 1 made the fullscreen Input Screen unreachable; this one deletes it. No behaviour change.
Commit by commit:
inputscreen/. A large minority of that package is used by the native input widget, so it moves first: the chat suggestions list and its readers, the metrics tree, the onboarding wide event, theInputModeWidgetbase class,InputScreenButtons,TabAttachmentTagSpanand the config resolver. Package renames only..maestro/input_screensuite with itsinputScreenTesttag (the valid-tags comment ine2e-maestro.ymlis updated).duckchat-apicontract cleanup.InputScreenActivityParams, the result codes and params, the browser-buttons config andBrowserAndInputScreenTransitionProviderall lost their last consumer in PR 1.DuckAiOnboardingEndCtaVariantis still used byCtaViewModel, so it moves to its own file in the same package. These are deletions only, no new or changed API surface.DUCK_CHAT_OPEN_AUTOCOMPLETE_EXPERIMENTALturned out to be orphaned.Four notes where I departed from the original plan, all deliberate:
InputScreenConfigResolveris deleted, not kept. Every member read intent extras that onlyInputScreenActivityever set, so on the native path all five resolved to constants (isTopOmnibartrue, henceuseTopBar()true; no installed apps; no voice launch; main buttons disabled). Keeping a class whose every method returns a constant is worse than inlining, so the two native call sites now use the constants directly:AutoComplete.Config(showInstalledApps = false)andOmnibarType.SINGLE_TOP.metric/nativeinput/, notmetrics/nativeinput/.duckchat-implalready has ametric/package, and two packages differing by one letter is a trap.RealDuckChatPixels.DUCK_CHAT_EXPERIMENTAL_OMNIBAR_BACK_BUTTON_PRESSEDin particular is fired straight fromInputModeWidget, so it survives until PR 3.app/lint-baseline.xmlentries are rewritten rather than removed. TheNoImplImportsInAppModuleviolations are unchanged, the onboarding files still import the wide event from an impl module, only the FQN moved. The tenduckchat-implbaseline entries pointing at deleted files are removed; six unrelated stale entries predate this branch and are left alone.Leftovers for the flag-retirement step:
duckchat-api's package is still calledinputscreeneven though it now holds three native-input contracts, and the moved metrics classes keep theirInputScreen*names. Whether the input-screen retention and discovery pixels should keep firing is a product question, not a mechanical one.Steps to test this PR
Install an internal build. Nothing should look or behave differently.
Native input widget
Suggestions (exercises the moved suggestions and reader packages)
Contextual sheet
Settings (exercises the moved discovery funnel)
Regression
UI changes
None.
Note
Low Risk
Large deletion-only cleanup after the fullscreen screen was already unreachable; native input paths retain the moved shared code with no new runtime entry points.
Overview
This PR deletes the fullscreen Input Screen after the prior step made it unreachable. No intended user-facing behavior change — search/chat input stays on the native omnibar widget.
Removed:
InputScreenActivityand its fragment stack (search/chat tabs, view model, session store, tab attachments, transition provider), related layouts and manifest entry, the full.maestro/input_screensuite, and dead API types (InputScreenActivityParams, result codes,BrowserAndInputScreenTransitionProvider, browser-buttons config).Relocated code still needed by the native widget: chat suggestions store/readers, discovery metrics under
metric/nativeinput/, onboarding wide event underwideevents/, and shared UI pieces (InputModeWidgetfamily, buttons, tab tag span).DuckAiOnboardingEndCtaVariantremains induckchat-apifor native onboarding CTAs.Housekeeping: onboarding imports updated to the new wide-event package;
e2e-maestro.ymldrops theinputScreenTesttag; lint baselines adjusted for moved FQNs and deleted files.Reviewed by Cursor Bugbot for commit 6f8f3b8. Bugbot is set up for automated code reviews on this repo. Configure here.