[material_ui] Document that TextField.decoration is merged with InputDecorationTheme - #12493
[material_ui] Document that TextField.decoration is merged with InputDecorationTheme#12493Treamz wants to merge 3 commits into
Conversation
…DecorationTheme TextField.decoration's dartdoc did not mention that the decoration passed here is combined with the ambient InputDecorationTheme via InputDecoration.applyDefaults, rather than replacing it. That makes it non-obvious why, for example, a theme-provided border survives an InputDecoration that simply omits one. Document the merge and point at the explicit opt-out. Addresses the documentation half of flutter/flutter#159629
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for TextField.decoration to clarify that it merges with the ambient InputDecorationTheme rather than replacing it, alongside bumping the package version to 1.0.1. Feedback on the documentation changes suggests correcting a comma splice, clarifying the return type of InputDecorationTheme.of as InputDecorationThemeData, and using a proper Dartdoc reference for [InputBorder.none].
| /// This decoration does not replace the ambient [InputDecorationTheme], it is | ||
| /// merged with it by [InputDecoration.applyDefaults]. Only the properties | ||
| /// that are left null here take their value from [InputDecorationTheme.of], | ||
| /// which in turn defaults to [ThemeData.inputDecorationTheme]. So, for | ||
| /// example, when the theme specifies a border, passing an [InputDecoration] | ||
| /// that leaves [InputDecoration.border] null does not remove that border; | ||
| /// pass `border: InputBorder.none` to opt out of it explicitly. |
There was a problem hiding this comment.
There are a couple of areas for improvement in this documentation paragraph:\n\n1. Grammar: The first sentence contains a comma splice: ...ambient [InputDecorationTheme], it is merged.... It should be split into two sentences or separated with a semicolon.\n2. Accuracy: The phrase take their value from [InputDecorationTheme.of] is slightly imprecise because InputDecorationTheme.of is a static method that returns InputDecorationThemeData. It is clearer to state that they take their values from the InputDecorationThemeData returned by InputDecorationTheme.of.\n3. Dartdoc Reference: Instead of using backticks for border: InputBorder.none, we can use a proper Dartdoc reference [InputBorder.none] to make it clickable and consistent with other references in the paragraph.
/// This decoration does not replace the ambient [InputDecorationTheme]. Instead,\n /// it is merged with it by [InputDecoration.applyDefaults]. Only the properties\n /// that are left null here take their value from the [InputDecorationThemeData]\n /// returned by [InputDecorationTheme.of], which in turn defaults to\n /// [ThemeData.inputDecorationTheme]. So, for example, when the theme specifies\n /// a border, passing an [InputDecoration] that leaves [InputDecoration.border]\n /// null does not remove that border; pass [InputBorder.none] to opt out of it\n /// explicitly.… InputBorder.none
material_ui sets 'release: batch: true' in ci_config.yaml, so the version and changelog validator rejects direct edits to CHANGELOG.md and to the pubspec version. Revert both and add a pending_changelogs entry instead.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
TextField.decoration's dartdoc describes what the decoration draws, but never mentions that the value passed here is combined with the ambientInputDecorationThemerather than replacing it.TextFieldbuilds its effective decoration withInputDecoration.applyDefaults(packages/material_ui/lib/src/text_field.dart), andapplyDefaultsonly substitutes properties that are null on the incomingInputDecoration— includingborder: border ?? theme.border(packages/material_ui/lib/src/input_decorator.dart). So a theme-provided border survives anInputDecorationthat simply omits one, which is surprising if you read the property docs alone. That confusion is what flutter/flutter#159629 reports.This adds a paragraph to the
decorationproperty documenting the merge and pointing at the explicit opt-out (InputBorder.none).This covers the documentation half of flutter/flutter#159629. The issue also proposes a
mergeWithTheme-style flag onInputDecoration; that is an API proposal and is deliberately left out of this PR, so the issue should stay open.Originally opened against
flutter/flutteras flutter/flutter#191260, and moved here after the code-freeze bot pointed out that Material changes now belong inmaterial_ui.Documentation-only change; no behavior change and no new tests.
Pre-launch Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. —material_uiuses batch release (release: batch: trueinci_config.yaml), so this adds apending_changelogsentry withversion: patchrather than editingpubspec.yamldirectly.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes. — same as above;CHANGELOG.mdis generated at batch release time.