[codex] Add auto-growing text inputs - #3
Conversation
|
@Matildevoldsen this is still draft and there are some conflicts now. Feel free to firm this up when you get a chance. Note that we've renamed the package and some namespaces. |
shanerbaner82
left a comment
There was a problem hiding this comment.
Review of the auto-grow work. One crash-level issue on Android, plus the branch is stale against main in a way that will silently revert shipped work on both platforms if the conflicts are resolved in this PR's favor.
Blocking
- Android crashes when
min_lines> the defaultedmax_lines(inline). - Rebase needed:
mainhas rewrittenNativeUITextInputCore.swift(+336 lines, multiline width fix +selectionEnabled) and grown theBaseTextInputbehavior region by five public methods sincef3b0731.
Worth fixing before merge
autoGrow(false)enables auto-grow.autoGrow()clobbers previously-set line bounds.- The iOS
auto_growmodifier appears to be a no-op. - The tests assert on source text rather than behavior.
Details inline.
| singleLine = !props.multiline, | ||
| singleLine = props.singleLine, | ||
| minLines = if (props.multiline || props.autoGrow) props.minLines else 1, | ||
| maxLines = if (props.multiline || props.autoGrow) props.maxLines else 1, |
There was a problem hiding this comment.
Crash: min_lines > max_lines throws on first composition.
parseTextInputProps derives minLines and maxLines independently and never clamps one against the other. With this hunk now forwarding both to BasicTextField, <native:bare-text-input auto-grow min-lines="8" /> produces min_lines=8 with max_lines falling back to 5. Compose's Modifier.heightInLines does require(minLines <= maxLines), so the screen throws IllegalArgumentException.
The Blade path reaches this directly: applyAttributes runs autoGrow() (setting max_lines = 5) and then applies the min-lines attribute, with no clamp in between. Same for ->multiline()->minLines(8).
iOS's new resolvedLineRange() clamps for exactly this reason — Android should do the same in parseTextInputProps (maxLines = max(maxLines, minLines)), which also closes the same pre-existing exposure in the outlined and filled renderers.
| multiline = p.getBool("multiline"), | ||
| maxLines = p.getInt("max_lines").let { if (it > 0) it else if (p.getBool("multiline")) 5 else 1 }, | ||
| autoGrow = p.getBool("auto_grow"), | ||
| maxLines = p.getInt("max_lines").let { if (it > 0) it else if (p.getBool("multiline") || p.getBool("auto_grow")) 5 else 1 }, |
There was a problem hiding this comment.
This is where the clamp belongs — maxLines is resolved here with no reference to minLines, and the next line resolves minLines with no reference to maxLines. Mirroring iOS's resolvedLineRange() (maxLines = max(maxLines, minLines)) fixes the crash for all three renderers at once.
| } else if multiline { | ||
| TextField(placeholder, text: $text, axis: .vertical) | ||
| .lineLimit(maxLines > 0 ? 1...maxLines : 1...5) | ||
| .lineLimit(resolvedLineRange(minLines: minLines, maxLines: maxLines)) |
There was a problem hiding this comment.
Stale base — merging as-is reverts main's multiline width fix.
This branch is off f3b0731; origin/main has since rewritten this file (+336 lines). Main's multiline branch already computes the identical clamp:
let lower = max(minLines, 1)
let upper = maxLines > 0 ? max(maxLines, lower) : max(5, lower)…and adds .frame(maxWidth: .infinity, alignment: .leading), with a comment explaining that a vertical-axis TextField reports ~0 intrinsic width when empty and collapses to the icon. This PR's replacement branch has the clamp but not the frame, so resolving the conflict in favor of the PR re-introduces the collapse bug. Main also added a selectionEnabled parallel branch this hunk knows nothing about.
Please rebase onto main before merging.
| } | ||
| } | ||
| .font(.system(size: textSize)) | ||
| .modifier(AutoGrowingInputModifier(enabled: multiline && autoGrow)) |
There was a problem hiding this comment.
auto_grow doesn't appear to have a distinct effect on iOS. AutoGrowingInputModifier only applies fixedSize(horizontal: false, vertical: true), but a TextField(axis: .vertical) with lineLimit(lower...upper) already sizes to its intrinsic height and caps at upper — so this is a no-op relative to plain multiline + min-lines/max-lines. The actual growth comes from the lineLimit change above, which main already has.
It's also attached to the whole Group, so secure + auto_grow applies it to a SecureField that has no line range at all.
Either demonstrate a case where it changes layout, or drop it and treat auto_grow as an Android-only hint.
| */ | ||
| abstract class BaseTextInput extends Element | ||
| { | ||
| use ConfiguresTextInputBehavior; |
There was a problem hiding this comment.
The trait extraction conflicts with methods added on main.
This PR deletes the Behavior and Sync mode regions and moves them into a concern. On main that same region has since grown autocapitalize(), keepFocusOnSubmit(), font(), selectionDebounceMs(), and onSelectionChange(), plus a Pint reformat (this PR still uses the old single-line { ... } brace style, which Pint will now reject). Resolving the conflict by taking this PR's side silently drops five shipped public API methods.
The refactor is also unrelated to the auto-grow feature — it'd be safer as its own PR after a rebase, so the feature diff stays reviewable.
| return $this; | ||
| } | ||
|
|
||
| public function autoGrow(int $minLines = 1, int $maxLines = 5): static |
There was a problem hiding this comment.
autoGrow(false) enables auto-grow.
Every other toggle in this trait is fn(bool $value = true). autoGrow(int $minLines = 1, int $maxLines = 5) breaks that pattern: with no strict_types, ->autoGrow(false) coerces to 0, max(1, 0) gives 1, and the method then unconditionally sets multiline = true and auto_grow = true.
So ->autoGrow($userWantsAutoGrow) turns a single-line input into a multiline auto-growing one precisely when the flag is false — and there's no way to disable auto-grow once it's set. Consider autoGrow(bool $value = true, ?int $minLines = null, ?int $maxLines = null), or a separate autoGrowLines().
| $this->inputProps['multiline'] = true; | ||
| $this->inputProps['auto_grow'] = true; | ||
| $this->inputProps['min_lines'] = $minLines; | ||
| $this->inputProps['max_lines'] = $maxLines; |
There was a problem hiding this comment.
These unconditional writes make the fluent API order-dependent: ->maxLines(10)->autoGrow() silently resets max_lines back to 5, while ->autoGrow()->maxLines(10) works. Only write min_lines/max_lines when the caller actually passed non-default arguments (see the nullable-args suggestion above), or document the overwrite clearly.
|
|
||
| expect($shared) | ||
| ->toContain('val autoGrow: Boolean') | ||
| ->toContain('autoGrow = p.getBool("auto_grow")') |
There was a problem hiding this comment.
The suite asserts on source text rather than behavior — every assertion is file_get_contents(...)->toContain('...'), including whitespace-sensitive strings like this one ('autoGrow = p.getBool("auto_grow")') and 'val autoGrow: Boolean'.
These pass even if autoGrow() writes the wrong prop keys, if applyAttributes never calls it, or if the Android renderer crashes on minLines > maxLines — and they break on any Pint or ktlint realignment.
A behavioral test would be more useful: build OutlinedTextInput::make()->autoGrow(2, 6), resolve the props, and assert auto_grow === true, min_lines === 2, max_lines === 6; plus a Blade-attribute case covering both auto-grow and autoGrow.
Summary
Adds first-class auto-growing multiline text input support for Native UI text inputs.
autoGrow(int $minLines = 1, int $maxLines = 5)API and Bladeauto-grow/autoGrowattribute supportmultiline,minLines, andmaxLinesbehavior compatibleauto_growprop through Android shared text input parsing and the chromelessBareTextInputRendererauto_growprop through SwiftUI and asks vertical text fields to use intrinsic height withfixedSize(horizontal: false, vertical: true)BaseTextInput.phpstays under 300 linesWhy
Chat composer style inputs need to grow naturally from one line up to a capped number of lines without each app estimating wrapped line counts in Blade/PHP. Native UI already has multiline inputs; this adds the explicit API and native hints needed for textarea-style auto-growth.
Validation
Passed:
Current upstream/fresh-clone test blockers, unrelated to this change: