Skip to content

Conversation

@HansMuller
Copy link
Contributor

The InputContainer widget arranges its child, hint text, label, and error text per the Material spec: https://material.google.com/components/text-fields.html. The Input widget is now a simple combination of InputField and InputContainer.

The InputContainer's layout depends on two boolean parameters that are not defined for Input: focused and isEmpty. They imply the location and text attributes for the input hint and label. The Input widget sets focus to true when it has the focus, and isEmpty to true when its value is the empty string.

Fixes #6421

const Duration _kTransitionDuration = const Duration(milliseconds: 200);
const Curve _kTransitionCurve = Curves.fastOutSlowIn;

/// A simple text field.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some more prose here describing what this means.

///
/// See also:
///
/// * [Input]
Copy link
Contributor

Choose a reason for hiding this comment

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

add a brief explanation of why you're pointing to these, as in * [Input], which does this and that.

///
/// Requires one of its ancestors to be a [Material] widget.
/// Displays the visual elements of a material design text field. This
/// widget's child is the input field widget itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe s/text/form/ here, so people think they can use this for things other than text fields?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only the first line is visible in the dartdoc overview pages, and I'm worried these two sentences make it sounds text-field specific (the next sentence explains that it's not, but that won't be visible).

// InputFormField below.
Input({
/// * [InputField]
/// * [Input]
Copy link
Contributor

Choose a reason for hiding this comment

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

, which...

///
/// See also:
///
/// * [InputField]
Copy link
Contributor

Choose a reason for hiding this comment

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

, which...

///
/// By default, the input uses a keyboard appropriate for text entry.
//
// Note: If you change this constructor signature, please also update
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Note://

}
),
),
];
Copy link
Contributor

Choose a reason for hiding this comment

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

why is there a Builder in the build function above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This the comment that appears here. Maybe it's not clear?

// Since the focusKey may have been created here, defer building the
// RawInput until the focusKey's context has been set. This is necessary
// because the RawInput will check the focus, like Focus.at(focusContext),
// when it builds.

_inputFieldKey.currentState?.requestKeyboard();
},
child: child,
child: new Builder(
Copy link
Contributor

Choose a reason for hiding this comment

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

why a Builder?

Copy link
Contributor Author

@HansMuller HansMuller Nov 16, 2016

Choose a reason for hiding this comment

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

I'll add this comment:

Since the focusKey may have been created here, defer building the InputContainer until the
focusKey's context has been set. This is necessary because we're passing the value of
Focus.at(focusKey.currentContext, ...) along to the InputContainer.

@Hixie
Copy link
Contributor

Hixie commented Nov 16, 2016

This is fantastic.

@Hixie
Copy link
Contributor

Hixie commented Nov 16, 2016

I have only the most minor comments, mostly about docs. LGTM.

@HansMuller HansMuller merged commit a7360d2 into flutter:master Nov 16, 2016
@HansMuller HansMuller deleted the input_refactor_round2 branch November 16, 2016 18:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 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.

Factor out the material form control part of Input so it can be reused in non-text-editing scenarios

2 participants