fix(keyboard-actions): host the toolbar in the root overlay - #176
Conversation
The toolbar is positioned in SCREEN terms: `bottom: viewInsets.bottom`, so that it sits on top of the keyboard. It was inserted into the NEAREST overlay, which is measured in its own box, and a nested overlay is not the screen. Measured on an iPhone in an app whose page host owns an Overlay inside a scroll view: two overlays, one 402x874 (the screen) and one 402x2146 (the scrolled content), both reporting the same 335pt inset. The entry went into the second, so `bottom: 335` placed the toolbar 335pt from the bottom of the CONTENT, roughly 1300pt below the viewport. It was present in the semantics tree the whole time and nowhere on the screen, which is the worst shape a bug like this can take: a tree assertion passes and the user still has no button. `rootOverlay: true` is the fix, and the new test is the shape that produced it: an Overlay nested in a 2400pt scroll view, asserting the toolbar's rect stays within the screen. It reports 2400 against a 600pt screen without the fix.
|
Warning Review limit reached
Next review available in: 53 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesKeyboard toolbar placement
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The change keeps the keyboard toolbar above the keyboard instead of positioning it relative to scrolled content, fixing an off-screen user-facing issue. Merge readiness remains pending explicit results for analysis, formatting, and required coverage checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 similar comments
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. The one-line fix is correct - the toolbar is positioned in screen terms so the root overlay is the right host - and the only gap is the missing CHANGELOG entry the repo requires. Major
Minor
The class docstring (line 11, "renders an above-keyboard toolbar in the app's TestsThe new Checks I ran
|
…l and changelog The fix changed WHICH overlay hosts the toolbar, and nothing outside the implementation said which one it was. The class doc claimed "the app's Overlay" without qualification, `doc/widgets/w-keyboard-actions.md` was silent, and the skill's OverlayEntry section covered the theme-context caveat while never mentioning that host choice is a second, separate decision. Skill version 2.12.0.
Resolves the CHANGELOG conflict both entries created by claiming the first bullet under `### Fixed`. Kept both, flex first because it landed first; no wording changed on either side. `skills/wind-ui/SKILL.md` merged clean since both branches set the same 2.12.0.
Minor release, not a patch. The Return-key default on a single-line `WInput` moved from `TextInputAction.next` to `.done` (#177), which changes behaviour in every consumer that never passed `textInputAction`. Consumers are pinned with `^1.3.0`, so a patch would have landed that on everyone at the next `pub upgrade` with the version number saying nothing had changed. A minor still reaches them automatically, so the two layout fixes are not withheld, but the number now says to read the notes. Ships with those two fixes: a `justify-between` row no longer splits its width between a child that asked to grow and siblings that did not (#175), and `WKeyboardActions` hosts its toolbar in the root overlay so it lands on the keyboard rather than hundreds of points below the viewport (#176). Plus the CI fix that had every open PR red on a dirty checkout (#178) and the codeql-action bump (#174). Bump pubspec.yaml 1.3.0 -> 1.4.0 and promote ## [Unreleased] to ## [1.4.0] - 2026-08-21 in CHANGELOG.md, with the [1.4.0] link reference and the [Unreleased] compare link redirected to 1.4.0...HEAD. Sync example/pubspec.yaml, the dartdoc_options.yaml source-link tag, and the llms.txt version string. Move the wind-ui skill to the 1.4 line: the nine reference H1s plus the SKILL.md description and version marker.
1 participant
What
WKeyboardActionsinserts its toolbar intoOverlay.of(context, rootOverlay: true)instead of the nearest overlay.Why
The toolbar is positioned in screen terms,
bottom: viewInsets.bottom, so it sits on top of the keyboard. A nested overlay is measured in its own box, so hosting it there positions it relative to content rather than the screen.Measured on an iPhone, in an app whose page host owns an
Overlayinside a scroll view: two overlays, one 402x874 (the screen) and one 402x2146 (the scrolled content), both reporting the same 335pt bottom inset. The entry went into the second, sobottom: 335put the toolbar 335pt from the bottom of the CONTENT, roughly 1300pt below the viewport.It was in the semantics tree the whole time and nowhere on the screen, which is the worst shape this bug can take: a tree assertion passes and the user still has nothing to press. Confirmed fixed on the device, with the button now sitting directly above the keyboard.
Testing
Overlaynested in a 2400pt scroll view, asserting the toolbar's rect stays within the screen height. Without the fix it reports2400.0against a600.0screen.Summary by CodeRabbit
Bug Fixes
Tests
Review pass (2026-08-21)
The original commit touched
lib/andtest/only, so the post-change syncCLAUDE.mdrequires was missing on four surfaces, and the interesting part of this fix is exactly the part nothing outside the implementation recorded: WHICH overlay hosts the toolbar.Overlay" with no qualification, which is the very ambiguity that caused the bug. It now names the root overlay and why.doc/widgets/w-keyboard-actions.mdwas silent on overlay hosting. It now carries a note at the end of Basic Usage, including the reassurance that a consumer needs noOverlayof their own.skills/wind-ui/references/theme.mdsection 12 coversOverlayEntryand stops at the theme-context caveat. Its snippet'sOverlay.of(context)line was left alone deliberately: it demonstrates a content-anchored menu, where the nearest overlay is the right answer, so rewriting it torootOverlay: truewould teach the wrong default. A paragraph after it names host choice as the second, separate decision and uses this bug as the worked example.CHANGELOG.mdgained the### Fixedentry.README.mdneeds nothing (no roster change) and no example page is needed (no new token, and the bug only reproduces against a real keyboard inset).Gates
Run locally on the branch tip:
dart format --set-exit-if-changed: 0 changed.dart analyze lib/ test/ example/lib/: No issues found../tool/coverage.sh 90: 1693 passed, 1 skipped, coverage 94.8%.python3 tool/check-docs.py: 0 issues across 72 doc pages.Lint & Teststays red here until #178 lands onmaster:flutter pub getrewritesanalysis_options.yamlmid-run anddart pub publish --dry-runthen fails on a dirty checkout. Nothing on this branch causes it.