New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a maxLines parameter for multiline Input. #6310
Conversation
If maxLines is 1, it's a single line Input that scrolls horizontally. Otherwise, overflowed text wraps and scrolls vertically, taking up at most `maxLines`. Also fixed scrolling behavior so that the Input scrolls ensuring the cursor is always visible. Fixes #6271
_caretPrototype = new Rect.fromLTWH(0.0, _kCaretHeightOffset, _kCaretWidth, lineHeight - 2.0 * _kCaretHeightOffset); | ||
_selectionRects = null; | ||
_textPainter.layout(maxWidth: _maxContentWidth); | ||
size = new Size(constraints.maxWidth, constraints.constrainHeight(math.max(lineHeight, _textPainter.height))); | ||
size = new Size(constraints.maxWidth, constraints.constrainHeight( | ||
_textPainter.height.clamp(lineHeight, lineHeight * _maxLines))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline before final )
Size contentSize = new Size(_textPainter.width + _kCaretGap + _kCaretWidth, _textPainter.height); | ||
if (onPaintOffsetUpdateNeeded != null && (size != oldSize || contentSize != _contentSize)) | ||
onPaintOffsetUpdateNeeded(new ViewportDimensions(containerSize: size, contentSize: contentSize)); | ||
assert(_selection != null); // valid assumption? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should determine if it is before checkin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(one option is to just say that it is, and treat it as a bug if we find it isn't later)
@@ -485,7 +486,9 @@ class ScrollableState<T extends Scrollable> extends State<T> with SingleTickerPr | |||
|
|||
if (duration == null) { | |||
_stop(); | |||
_setScrollOffset(newScrollOffset, details: details); | |||
// scheduleMicrotask(() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops
// Used for state changes that sometimes occur during a build phase. If so, | ||
// we skip calling setState, as the changes will apply to the next build. | ||
void _setStateMaybeDuringBuild(VoidCallback fn) { | ||
if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.persistentCallbacks) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is really valid. What if your ancestor is setting it on you during their build, for instance?
But I'm fine with checking this in. It will work around a bug that people are complaining about, and this code is all getting rewritten in my refactor anyway.
cc @abarth who may have opinions about this in particular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's ugly and I'm hoping it won't be necessary in the Scrollable rewrite. I'll add a note in the comments.
LGTM with tests. |
Offset caretOffset = _textPainter.getOffsetForCaret(caretPosition, _caretPrototype); | ||
// This rect is the same as _caretPrototype but without the vertical padding. | ||
return new Rect.fromLTWH(0.0, 0.0, _kCaretWidth, lineHeight).shift(caretOffset + _paintOffset); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed the caret rect I use here, dropping the vertical padding so that we scroll all the way to the edge when it moves.
@mpcomplete I'm landing this for you to avoid a merge conflict in |
If maxLines is 1, it's a single line Input that scrolls horizontally.
Otherwise, overflowed text wraps and scrolls vertically, taking up at
most
maxLines
.Also fixed scrolling behavior so that the Input scrolls ensuring the
cursor is always visible.
Fixes #6271