Skip to content

Adding supportingTextPadding to InputDecorator and InputDecorationTheme#183582

Open
MohanadAbdallah-mv wants to merge 18 commits into
flutter:masterfrom
MohanadAbdallah-mv:master
Open

Adding supportingTextPadding to InputDecorator and InputDecorationTheme#183582
MohanadAbdallah-mv wants to merge 18 commits into
flutter:masterfrom
MohanadAbdallah-mv:master

Conversation

@MohanadAbdallah-mv
Copy link
Copy Markdown

@MohanadAbdallah-mv MohanadAbdallah-mv commented Mar 12, 2026

This PR introduces the supportingTextPadding property to InputDecorator, InputDecoration, and InputDecorationThemeData.
While originally conceived as errorPadding, this property was renamed to supportingTextPadding during review to align with the Material Design 3 specification, as it controls the padding for the entire supporting text row (which includes helper text, error text, and counter widgets).
Additionally, this PR addresses layout, intrinsic sizing, and code consistency concerns raised during the review process:

Key Features & Refactors:

  1. Added supportingTextPadding: Allows complete, independent control over the padding of the supporting text row (helper/error/counter).
  2. Decoupled Layout Constraints: Introduced supportingTextConstraints in _layout() to bound the supporting text row independently of contentConstraints, ensuring custom paddings do not break or shift the layout of inline prefix/suffix widgets.
  3. Fixed Intrinsic Height Mismatches: Corrected computeMinIntrinsicHeight() to compute supportingTextWidth separately from the primary width deflation path, ensuring intrinsic height calculations accurately predict wrapped text heights.
  4. Unified Terminology: Renamed all legacy internal subtext references (e.g. _SubtextSize, _computeSubtextSizes, subtextHeight) to supportingText to match the public property name and the M3 spec.

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
Fixes #175834

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 12, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an errorPadding property to InputDecorator and InputDecorationThemeData, allowing for separate padding for error text. The implementation looks correct and includes relevant tests. I've made a couple of suggestions to improve the documentation and maintain code consistency.

Comment thread packages/flutter/lib/src/material/input_decorator.dart Outdated
Comment thread packages/flutter/lib/src/material/input_decorator.dart Outdated
@MohanadAbdallah-mv
Copy link
Copy Markdown
Author

Hi @dkwingsmt
it seems like there was a slight delay , is there any other thing I can do to ease up the review process ?

@dkwingsmt dkwingsmt self-requested a review March 25, 2026 18:33
Copy link
Copy Markdown
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

I'm ok with the API design. Minor problems with the implementation. If you agree with the comments, new tests should be added accordingly.

case TextDirection.ltr:
start = contentPadding.start + _boxSize(icon).width;
startError = (errorPadding?.start ?? contentPadding.start) + _boxSize(icon).width;
end = overallWidth - contentPadding.end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be affected by errorPadding? Also the rtl case below, where the end padding should have been more critical.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have just noticed i misunderstood the end padding in rtl case, what previously happened was that the counter , error and helper padding was affected by the content padding , as far as i know endPadding will take effect on the counter which in the current approach is handled with the old contentPadding so is that the case you mentioned and should the counter padding should be handled with end padding just as the contentPadding did before?
or helper and error padding should be separated from content Padding for now and later on maybe we can consider separating counter Padding as well to add more flexibility

TL;DR
do we add endError to control the counter padding along with the errorPadding or should it be handled with content Padding?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think it makes the most sense for this new padding to control the counter's alignment as well. If we only decouple the error text from contentPadding but leave the counter tied to it, they might become visually misaligned. Let's use the end value of the new padding for the counter.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree with you on this part , in the last commit i separated counter padding from the contentPadding

_boxParentData(helperError).offset = Offset(
start + decoration.inputGap,
startError + decoration.inputGap,
subtextBaseline - helperErrorBaseline,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this padding is also used for helper, I think we should also mention helper somewhere. I prefer changing the name to helperErrorPadding or errorHelperPadding, but if you prefer errorPadding, we can at least mention this in the property document.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, should we also consider vertical padding here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

you are right about the naming my main focus was on the error itself since the first issue #181755 was introduced , also i don't know if we should consider vertical padding or not, i guess then we will have to deal with the size itself of the size of the widget below in that method

927|  _SubtextSize? _computeSubtextSizes({
928| required BoxConstraints constraints,
929| required ChildLayouter layoutChild,
930|   required _ChildBaselineGetter getBaseline,
931|  }) {

and as mentioned before this will take effect on the bottom row which have error, counter and helper.
if we considered control over vertical padding in that case maybe we can use error vertical padding instead of the default subtextGap value if vertical padding like that

final double topPadding = errorPadding?.top ?? subtextGap;
final double bottomPadding = errorPadding?.bottom ?? 0.0;

that will take effect on the whole row and using the old subtextGap will not break any old application but is adding such flexibility for the developer is good considering UI guidelines also do we need to consider more flexibility and separate padding values of helper , text and counter padding .
my opinion if we going to consider vertical padding maybe
A- we can start padding with the whole row so adding vertical padding will affect counter, error and helper
B- we can separate vertical padding for each one of them in that case we will consider helper Padding , counter padding and error padding values
C- we can stick with the current hardcoded subtextGap value that controls the vertical padding of the bottom row
from my perspective "C" is the current approach, we can consider "A" as our next plan and maybe we can consider "B" as optional future plan.
let me know what do you think .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should directly go with Option A. It provides a much cleaner and more predictable API if this single property controls the EdgeInsets (both horizontal and vertical) for the entire subtext row (error, helper, and counter), which is also more intuitive for developers when they see this property.

We can still ensure backward compatibility with option A: When the new padding is null, we can fall back to the legacy behavior.

In fact, if we go with option C now, migrating to option A will be a breaking change, since vertical padding has been ignored but will be effective in the future.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, in terms of name, since it now includes counter as well, we might call it supportingTextPadding. Although Flutter has not started using the name "supporting text" yet, it is an official name in the M3 spec (https://m3.material.io/components/text-fields/specs ).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

following the M3 spec everything looks clear now regarding the naming and how i handled the padding in the new commit,
counter , helper and error text will be aligned using supportingTextPadding .
let me know if there's anything is missing

Example with new API:

 supportingTextPadding: EdgeInsets.only(left: 10,right: 16,top: 10,bottom: 5),
 contentPadding: EdgeInsets.only(left: 12,right: 12,top: 12,bottom: 12),
image image

@dkwingsmt dkwingsmt self-requested a review April 1, 2026 18:25
Copy link
Copy Markdown
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Sorry for late review. I did another round of full review and these are the only thing I can find. Generally minor comments.

Also, there are still usages of subtextGap in this file (namely in computeMinIntrinsicHeight). The subtextGap field should no longer be used except as default values; all previous usages should be converted to something related to supportingTextPadding.

subtextBaseline - helperErrorBaseline,
);
if (counter != null) {
_boxParentData(counter).offset = Offset(
end - counter.size.width - decoration.inputGap,
endSupporting - counter.size.width - decoration.inputGap,
// endSupporting - counter.size.width - decoration.inputGap,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Extra line?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have updated computeMinIntrinsicHeight to include the supportingTextPadding vertical and fallback to default value subtextGap in case of supportingTextPadding is not provided

Comment thread packages/flutter/lib/src/material/input_decorator.dart Outdated
Comment on lines +3943 to +3944
/// if [supportingTextPadding] is null, the value of [contentPadding] will be used for both supporting text widgets [InputDecoration.helper] , [InputDecoration.counter]
/// and [InputDecoration.error].
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// if [supportingTextPadding] is null, the value of [contentPadding] will be used for both supporting text widgets [InputDecoration.helper] , [InputDecoration.counter]
/// and [InputDecoration.error].
/// If [supportingTextPadding] is null, the value of [contentPadding] will be used
/// for both supporting text widgets [InputDecoration.helper], [InputDecoration.counter],
/// and [InputDecoration.error].

It should also clearly state that when non-null, it completely overrides the default behavior (including subtextGap).

final double bottomHeight = math.max(counterAscent, helperErrorHeight) + subtextGap;
final double subtextHeight = math.max(counterSize.height, helperErrorHeight) + subtextGap;
math.max(counterAscent, getBaseline(helperError, helperErrorConstraints)) + topPadding;
final double bottomHeight = math.max(counterAscent, helperErrorHeight) + bottomPadding;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be

Suggested change
final double bottomHeight = math.max(counterAscent, helperErrorHeight) + bottomPadding;
final double bottomHeight = math.max(counterAscent, helperErrorHeight) + topPadding + bottomPadding;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I misunderstood bottomHeight as the space including supporting text and the space below it , not as the "bottom" refers to the whole bottom widgets below the main container .

I have fixed it in e720eaae720eaa as well as giving the docs more context.

I guess we covered all the points now, let me know if there's any more concerns , do we need to provide specific test for computeMinIntrinsicHeight ?

@dkwingsmt since now there's code freeze what is the suggested next step ?

Comment on lines +954 to +955
// topPadding and bottomPadding control the vertical padding of the row containing [helper, error, counter] widgets
// in case of null values it will fallback to the legacy code to avoid breaking any established codebase
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Phrasing like "fallback to the legacy code" isn't ideal for future readers of the codebase because it doesn't describe the actual behavior. It's better to explicitly state what happens when the value is null. Something like:

    // The vertical padding around the row containing the helper, error, and counter widgets.
    // Defaults to `subtextGap` at the top and 0.0 at the bottom when `supportingTextPadding` is null.

- Update doc comments to clarify override behavior of subtextGap
- Fix bottomHeight to include topPadding
@dkwingsmt dkwingsmt self-requested a review May 6, 2026 18:20
@dkwingsmt dkwingsmt added the CICD Run CI/CD label May 7, 2026
Copy link
Copy Markdown
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM except for minor checks.

Also the analyzer shows that there are some format errors in both files, which can be solved by dart format ..

Comment thread packages/flutter/lib/src/material/input_decorator.dart Outdated
/// within a [Theme].
/// * [InputDecoration.visualDensity], which can override this setting for a
/// given decorator.
/// {@macro flutter.material.inputDecoration.visualDensity}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This macro isn't defined. Did you write it by mistake?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wrote macro by mistake and removed it on latest commit, also reformatted both files on current PR Flutter version

Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>
@github-actions github-actions Bot removed the CICD Run CI/CD label May 11, 2026
@dkwingsmt dkwingsmt added the CICD Run CI/CD label May 13, 2026
dkwingsmt
dkwingsmt previously approved these changes May 14, 2026
Copy link
Copy Markdown
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM. (Also to trigger google tests)

Copy link
Copy Markdown
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

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

Hi @MohanadAbdallah-mv, thank you for your contribution and patience! I left a few small comments but overall this looks like a reasonable addition!

);
final BoxConstraints contentConstraints = containerConstraints.deflate(
EdgeInsetsDirectional.only(
start: contentPadding.start + decoration.inputGap,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I found a bug here, contentConstraints are used for subtextSize but never consider supportingTextPadding override. This potentially causes overlaps where the supporting text is laid out with the size of the contentPadding instead of that of the supportingTextPadding.

Image
Center(
  child: Container(
    width: 300.0,
    decoration: BoxDecoration(
      border: Border.all(color: Colors.red, width: 2.0),
    ),
    child: const InputDecorator(
      decoration: InputDecoration(
        filled: true,
        labelText: 'Enter Text',
        supportingTextPadding: EdgeInsets.symmetric(
          horizontal: 80.0,
        ),
        helperText:
            'This is helper text that is long enough to wrap if constrained correctly but overflows instead.',
        helperMaxLines: 3,
      ),
    ),
  ),
),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @Renzo-Olivares , Thank you for addressing these concerns.
contentConstraints was passed to _computeSubtextSizes
I'd suggest going with separate constraints for supportingText/subText rather relying on changing the original contentConstraints because prefix and suffix depends on contentConstraints as well.
so my suggestion includes adding

final BoxConstraints subTextConstraints = containerConstraints.deflate(
      EdgeInsetsDirectional.only(
        start: (supportingTextPadding?.start??contentPadding.start) + decoration.inputGap ,
        end: (supportingTextPadding?.end??contentPadding.end) + decoration.inputGap,
      ),
    );

with a safe fallback to contentPadding and use it for _computeSubtextSizes like this

final _SubtextSize? subtextSize = _computeSubtextSizes(
      constraints: subTextConstraints,
      layoutChild: layoutChild,
      getBaseline: getBaseline,
    );    

But a small concern about the naming :
do we go with subTextConstraints to match the legacy naming or supportingTextConstraints to match M3 spec?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should probably change all references from subText to supportingText to match the material spec and so we do not have two names for the same concept which may be confusing to readers.

I suggest the following:

In one commit implement as subTextConstraints, and in the next commit change all references of subText to supportingText. That will make it easier to review since there are a few references of subText.

final double suffixIconHeight = _minHeight(suffixIcon, width);
final double suffixIconWidth = _minWidth(suffixIcon, suffixIconHeight);

width = math.max(width - contentPadding.horizontal - decoration.inputGap * 2, 0.0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably take into account supportingTextPadding for counterHeight and helperErrorAvailableWidth.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'd suggest introducing supportingTextWidth right before calculating prefix/suffix icon sizes, since the subtext row (error, helper, and counter) is not inline with the prefix/suffix area., just to avoid the miscalculation conflicts and it would be like this

    width = math.max(width - iconWidth, 0.0);
    final double supportingTextWidth = math.max(width - (supportingTextPadding?.horizontal ?? contentPadding.horizontal) - decoration.inputGap * 2, 0.0);

    final double prefixIconHeight = _minHeight(prefixIcon, width);
    final double prefixIconWidth = _minWidth(prefixIcon, prefixIconHeight);
    final double suffixIconHeight = _minHeight(suffixIcon, width);
    final double suffixIconWidth = _minWidth(suffixIcon, suffixIconHeight);

    width = math.max(width - contentPadding.horizontal - decoration.inputGap * 2, 0.0);
    // supportingTextWidth  used instead of width
    final double counterHeight = _minHeight(counter, supportingTextWidth);
    final double counterWidth = _minWidth(counter, counterHeight);

    final double counterPadding = counter != null ? _kSubtextCounterPadding : 0.0;
    // supportingTextWidth  used instead of width 
    final double helperErrorAvailableWidth = math.max(supportingTextWidth - counterWidth - counterPadding, 0.0);

Let me know what do you think.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good to me!

/// given decorator.
final VisualDensity? visualDensity;

/// The padding applied to the supporting text row.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: consider making this a template and re-use it in this file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also I wonder if we should exclude subtextGap since I don't think its part of the public API, so mentioning it is potentially leaking implementation detail. Just mentioning vertical gap should be fine.

Copy link
Copy Markdown
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

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

@MohanadAbdallah-mv, left a few more comments. We should probably also update the PR title/description to better match the current implementation.

@github-actions github-actions Bot removed the CICD Run CI/CD label May 20, 2026
@MohanadAbdallah-mv MohanadAbdallah-mv changed the title Adding errorPadding property to InputDecorator and InputDecorationThe… Adding supportingTextPadding to InputDecorator and InputDecorationTheme May 20, 2026
@MohanadAbdallah-mv
Copy link
Copy Markdown
Author

@Renzo-Olivares Everything should be up to date now , current implementation follow the plan we discussed earlier including the documentation update, PR title/description and replacing subtext references from packages/flutter/lib/src/material/input_decorator.dart , but there are still other files that still uses subtext.
Do we take initiative to replace it in that same PR or will it be handled in a separate one?

@Renzo-Olivares
Copy link
Copy Markdown
Contributor

@MohanadAbdallah-mv I did a quick search and the only other reference I could find is form.dart, is that what you're referring to? If so I think it's fine to change that reference to subtext as well. Are there others I missed?

@MohanadAbdallah-mv
Copy link
Copy Markdown
Author

@Renzo-Olivares yes , along with form.dart you can find subtextgap in text_field_test.dart and subtext in input_decorator_test.dart.
I will make sure to update these as well.

@github-actions github-actions Bot added the a: text input Entering text in a text field or keyboard related problems label May 21, 2026
@dkwingsmt dkwingsmt self-requested a review May 27, 2026 18:05
Copy link
Copy Markdown
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

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

@MohanadAbdallah-mv thank you for your quick changes and patience! Left a few more comments that I noticed.

this.alignLabelWithHint = false,
this.constraints,
this.visualDensity,
this.supportingTextPadding,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The InputDecorationTheme class should also add support for the new supportingTextPadding property. While InputDecorationThemeData has been correctly updated to include supportingTextPadding, the InputDecorationTheme class (which is an InheritedTheme wrapping InputDecorationThemeData) has not.

@@ -5602,7 +5689,8 @@ class InputDecorationThemeData with Diagnosticable {
other.alignLabelWithHint == alignLabelWithHint &&
other.constraints == constraints &&
other.disabledBorder == disabledBorder &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not from this PR, but this check is redundant as other.disabledBorder == disabledBorder is already checked on line 5685.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Renzo-Olivares I have added the missing property supportingTextPadding to InputDecorationTheme class and created separate PR for the redundant check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Theme-level option to align error text with the text field border (remove default indent/padding)

4 participants