TextArea: grow by actual wrapped line count while editing (#4854)#5206
Open
shai-almog wants to merge 1 commit into
Open
TextArea: grow by actual wrapped line count while editing (#4854)#5206shai-almog wants to merge 1 commit into
shai-almog wants to merge 1 commit into
Conversation
Collaborator
Author
|
Compared 128 screenshots: 128 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Contributor
Cloudflare Preview
|
shai-almog
force-pushed
the
fix-4854-growbycontent-wrap-clip
branch
from
3e7b350 to
a677a8c
Compare
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 124 screenshots: 124 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 70 screenshots: 70 matched. Benchmark Results
Build and Run Timing
|
Collaborator
Author
|
Compared 128 screenshots: 128 matched. Benchmark Results
Detailed Performance Metrics
|
A growByContent multi-line TextArea was clipping the previous row while being edited: text would wrap onto a new visual line but the field would not grow to follow it until focus moved away. The during-editing grow gate (#4741) decided whether to revalidate using estimateLineCount(), a character-column prediction (string length vs getColumns()). The real renderer wraps using font metrics against getWidth() - horizontalPadding, so with a proportional font text wraps a row earlier than the character count predicts. The estimate undercounted, revalidateLater() did not fire, and the field stayed one row short. Leaving the field ran a normal layout that used the accurate getLines(), which is why the size corrected itself. Fix: drop the wrap prediction entirely. Any growth in text length can push content onto an extra wrapped row, so textMightGrowByContent() now triggers on a length increase (or an added hard newline) and lets the layout's getLines() determine the actual size. revalidateLater() coalesces into a single layout per paint, so erring toward an extra revalidate is cheap and, crucially, never under-grows. The gate stays width- and EDT-independent and does not call getLines() from inside setText(), avoiding any layout re-entrancy. The bogus estimateLineCount() helper is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shai-almog
force-pushed
the
fix-4854-growbycontent-wrap-clip
branch
from
a677a8c to
2d9bfb8
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
1 participant
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.
Problem
Follow-up to #4859. As reported in the last comment on #4854, a
growByContentmulti-lineTextAreaclips the previous row while being edited: text wraps onto a new visual line but the field doesn't grow to follow it. Moving focus to another field corrects the size.Root cause
The during-editing grow gate added in #4741 (
textMightGrowByContent) decided whether torevalidateLater()usingestimateLineCount— a character-column prediction (string length vsgetColumns()). The actual renderer (initRowString/getLines()) wraps using real font metrics againstgetWidth() - horizontalPadding. With a proportional font, text wraps onto a new row before the raw character count reachescols, so the estimate undercounted,revalidateLater()never fired, and the field stayed a row short. Leaving the field runs a normal layout that uses the accurategetLines()— which is why it self-corrects (broken during editing, fine after).Fix
Drop the wrap prediction. The insight: over-triggering
revalidateLater()is harmless — it coalesces into one layout per paint, and that layout already computes the true size viagetLines()— while under-triggering is the bug. SotextMightGrowByContentnow triggers on any text-length increase (or an added hard newline) and lets the layout decide the real row count.This is deliberately conservative and:
getLines()from insidesetText(), so there's no layout re-entrancy / recursion risk,estimateLineCounthelper.Testing
mvn -Plocal-dev-javase -DunitTests -pl :codenameone-core-unittests test -Dtest=TextAreaTest— all 42 pass locally under JDK 8, includingtestGrowByContentRevalidatesParentWhenRowsGrowDuringEditing(which fails on an earliergetLines()-based draft because its TextArea is never laid out, i.e. width 0).🤖 Generated with Claude Code