Skip to content

fix(core): dismiss keyboard after Locator.fill()#214

Open
ebbsagar wants to merge 2 commits into
mobile-next:mainfrom
ebbsagar:Issue-154
Open

fix(core): dismiss keyboard after Locator.fill()#214
ebbsagar wants to merge 2 commits into
mobile-next:mainfrom
ebbsagar:Issue-154

Conversation

@ebbsagar

@ebbsagar ebbsagar commented Jun 27, 2026

Copy link
Copy Markdown

Related issues

This PR implements:

Summary

Locator.fill() now automatically dismisses the soft keyboard after text entry on mobile, making follow-up interactions (especially taps on elements below the focused input) reliable without requiring an explicit keyboard dismissal step.

The problem this solves

After calling .fill() on an input, the soft keyboard stays open. On Android this can cover interactive elements lower on the screen, such as submit buttons. The next .tap() may then hit the keyboard area or keep focus on the currently active input instead of activating the intended control.

The existing workaround (screen.goBack() after every fill) adds boilerplate and is easy to forget, leading to flaky tests.

What this PR does

packages/protocol/src/driver.ts

  • Added optional dismissKeyboard?(): Promise<void> method to the MobilewrightDriver interface. Optional so drivers without keyboard support can simply omit it.

packages/driver-mobilecli/src/driver.ts and packages/driver-mobilenext/src/driver.ts

  • Implemented dismissKeyboard() with a safety check:
    • Scans the view hierarchy for a focused text input before dismissing
    • Returns early if none is found, preventing accidental navigation
    • Android: sends back key event (keyboard-specific, not navigation)
    • iOS: sends Done key event (standard keyboard dismiss action)

packages/mobilewright-core/src/locator.ts

  • Locator.fill() now calls driver.dismissKeyboard?.() after driver.typeText() completes, enabled by default.
  • Added per-call opt-out: fill('text', { dismissKeyboard: false }) for users who want the keyboard to remain open.

e2e/src/android/facebook.keyboard.test.ts

  • Updated test title and comments to reflect the new auto-dismiss behavior.
  • Removed the wait(1000) and commented-out goBack() workaround.

Test plan

  • npm run build - clean (tsc -b)
  • npm run lint - clean (tsc --noEmit && eslint .)
  • npx playwright test packages/mobilewright-core/src/locator.test.ts - 49/49 passing
  • Real-device E2E (Android/Facebook login):
    • Default behavior (dismissKeyboard: true): keyboard dismissed -> tap succeeds
    • Opted out (dismissKeyboard: false): keyboard stays open -> tap fails -> confirms fix works
  • iOS not tested - the iOS path (pressKeys(['Done'])) was not verified on a real device. The reviewer should confirm this dismisses the keyboard correctly on iOS before merging.

Backwards compatibility

Change Compat
dismissKeyboard?() on driver interface Additive - optional method, existing drivers are unaffected
dismissKeyboard() with focused-input check Safer - only acts when a text input is focused
fill() auto-dismisses keyboard Behavioral change - opt-out per-call via { dismissKeyboard: false }
fill() signature widened with dismissKeyboard?: boolean Additive - existing calls continue to compile unchanged

Files changed

packages/protocol/src/driver.ts                 |  1 +
packages/driver-mobilecli/src/driver.ts          | 32 +++++++++++++++++++++++-
packages/driver-mobilenext/src/driver.ts         | 32 +++++++++++++++++++++++-
packages/mobilewright-core/src/locator.ts        |  6 ++++-
e2e/src/android/facebook.keyboard.test.ts       | 13 +++++-----
5 files changed, 72 insertions(+), 12 deletions(-)

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ebbsagar, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes and 24 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 809417b1-faab-49c7-bbe2-f33eb8251ff2

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd1956 and 36acbc2.

📒 Files selected for processing (4)
  • packages/driver-mobilecli/src/driver.ts
  • packages/driver-mobilenext/src/driver.ts
  • packages/mobilewright-core/src/locator.ts
  • packages/protocol/src/driver.ts

Walkthrough

An optional dismissKeyboard() method is added to the MobilewrightDriver interface. Locator.fill() now accepts dismissKeyboard (default true) and calls the driver hook after typing. MobilecliDriver and MobileNextDriver implement keyboard dismissal by checking for a focused text input in the current UI tree and sending platform-specific key presses when present.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: dismissing the keyboard after Locator.fill() on mobile.
Description check ✅ Passed The description accurately explains the keyboard auto-dismiss feature and the related driver and test changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/driver-mobilecli/src/driver.ts`:
- Around line 424-430: The dismissKeyboard() implementation in driver.ts is too
aggressive because it always sends BACK on Android and return on iOS, which can
change navigation or field contents instead of only hiding the soft keyboard.
Update the Driver.dismissKeyboard method to use a real keyboard-dismiss path or
add a keyboard-visible check before calling pressButton('BACK') or
pressKeys(['return']), so Locator.fill() does not introduce unintended side
effects.

In `@packages/driver-mobilenext/src/driver.ts`:
- Around line 417-423: The dismissKeyboard() implementation in driver.ts is too
aggressive because it always sends BACK on Android and return on iOS, which can
trigger navigation or input actions instead of only hiding the keyboard. Update
dismissKeyboard() to use a true “dismiss if open” approach in the Driver class,
and avoid relying on pressButton('BACK') or pressKeys(['return']) as
unconditional fallbacks. Make sure the behavior used by Locator.fill() only
closes the keyboard when present and does not change app state or field
contents.
🪄 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: bb84d3b9-3f50-4715-bb62-897bd6b28ca2

📥 Commits

Reviewing files that changed from the base of the PR and between 8f7b44c and 4601864.

📒 Files selected for processing (4)
  • packages/driver-mobilecli/src/driver.ts
  • packages/driver-mobilenext/src/driver.ts
  • packages/mobilewright-core/src/locator.ts
  • packages/protocol/src/driver.ts

Comment thread packages/driver-mobilecli/src/driver.ts
Comment thread packages/driver-mobilenext/src/driver.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant