Skip to content
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

Merged
merged 7 commits into from Oct 14, 2016
Merged

Add a maxLines parameter for multiline Input. #6310

merged 7 commits into from Oct 14, 2016

Conversation

mpcomplete
Copy link
Contributor

@mpcomplete mpcomplete commented Oct 13, 2016

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

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)));
Copy link
Contributor

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?
Copy link
Contributor

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

Copy link
Contributor

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(() {
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

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) {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@Hixie
Copy link
Contributor

Hixie commented Oct 13, 2016

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);
}
Copy link
Contributor Author

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.

@Hixie
Copy link
Contributor

Hixie commented Oct 14, 2016

@mpcomplete I'm landing this for you to avoid a merge conflict in input.dart

@Hixie Hixie merged commit c13a6e2 into flutter:master Oct 14, 2016
@mpcomplete mpcomplete deleted the maxlines branch October 20, 2016 18:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for setting max height on multiline text inputs
3 participants