Repository navigation
Conversation
Adds fontFeatures and fontVariations parameters to TextTheme.apply(), passing them through to each TextStyle.apply() call. Fixes flutter#192692
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
github-actions
Bot
added
a: text input
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for fontFeatures and fontVariations in the TextTheme.apply method, forwarding these properties to all text styles. It also includes unit tests for this new functionality. The review feedback points out that one of the new tests, which claims to verify that both properties are preserved when null, only asserts fontFeatures. It is recommended to update the test to cover fontVariations as well.
Comment on lines
+182
to
+192
| test('TextTheme apply null fontFeatures fontVariations preserves original', () { | ||
| const fontFeatures = <FontFeature>[FontFeature.enable('smcp')]; | ||
| final typography = Typography.material2018(); | ||
| final TextTheme baseTheme = typography.black.copyWith( | ||
| displayLarge: typography.black.displayLarge!.copyWith(fontFeatures: fontFeatures), | ||
| ); | ||
| final TextTheme theme = baseTheme.apply(); | ||
|
|
||
| expect(theme.displayLarge!.fontFeatures, fontFeatures); | ||
| expect(theme.displayMedium!.fontFeatures, isNull); | ||
| }); |
Contributor
There was a problem hiding this comment.
The test name indicates that it verifies both fontFeatures and fontVariations are preserved when they are null, but the test implementation only sets and asserts fontFeatures.
To ensure complete test coverage and verify that fontVariations is also correctly preserved, please update the test to include fontVariations as well.
test('TextTheme apply null fontFeatures fontVariations preserves original', () {
const fontFeatures = <FontFeature>[FontFeature.enable('smcp')];
const fontVariations = <FontVariation>[FontVariation('wght', 400.0)];
final typography = Typography.material2018();
final TextTheme baseTheme = typography.black.copyWith(
displayLarge: typography.black.displayLarge!.copyWith(
fontFeatures: fontFeatures,
fontVariations: fontVariations,
),
);
final TextTheme theme = baseTheme.apply();
expect(theme.displayLarge!.fontFeatures, fontFeatures);
expect(theme.displayLarge!.fontVariations, fontVariations);
expect(theme.displayMedium!.fontFeatures, isNull);
expect(theme.displayMedium!.fontVariations, isNull);
});References
- Verify test validity: Confirm that new or modified tests effectively catch the issue being fixed and would fail if the fix were reverted. (link)
Contributor
Mairramer
marked this pull request as draft
September 14, 2026 12:51
Merged
10 of 11 tasks
Author
|
This PR has been ported to flutter/packages in flutter/packages#12879. The ported PR includes:
Please close this PR and reference the new one in flutter/packages. |
nadeemmx
added a commit
to nadeemmx/packages
that referenced
this pull request
Sep 16, 2026
Adds fontFeatures and fontVariations parameters to TextTheme.apply(), matching the existing support in TextStyle.apply(). Fixes flutter/flutter#192692 Port of flutter/flutter#192730
nadeemmx
added a commit
to nadeemmx/packages
that referenced
this pull request
Sep 16, 2026
Ports flutter/flutter#192730 to flutter/packages. Adds List<FontFeature>? and List<FontVariation>? parameters to TextTheme.apply(), passing them through to each TextStyle.apply() call. This allows developers to apply font features and variations consistently across an entire text theme without modifying each TextStyle individually. Fixes flutter/flutter#192692
nadeemmx
added a commit
to nadeemmx/packages
that referenced
this pull request
Sep 16, 2026
Adds fontFeatures and fontVariations parameters to TextTheme.apply(), matching the existing support in TextStyle.apply(). Fixes flutter/flutter#192692 Port of flutter/flutter#192730
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 28, 2026
#12879) ## Description This PR ports flutter/flutter#192730 to flutter/packages. Adds `List<FontFeature>?` and `List<FontVariation>?` parameters to `TextTheme.apply()`, passing them through to each `TextStyle.apply()` call. This allows developers to apply font features and variations consistently across an entire text theme without modifying each `TextStyle` individually. ## Related Issue Fixes flutter/flutter#192692 Ports flutter/flutter#192730 ## Proposed Change - Added `List<FontFeature>?` and `List<FontVariation>?` parameters to `TextTheme.apply()` - Both default to `null`, preserving backward compatibility - Each of the 15 `TextStyle.apply()` calls within `TextTheme.apply()` now passes these parameters through ## Technical Details The `TextStyle` class already supports `fontFeatures` and `fontVariations` in its own `apply()` method. The `TextTheme.apply()` method was missing the ability to pass these through. This is a purely additive change - no existing behavior is modified. ### Use Case: Persian Digits ```dart final theme = Theme.of(context).textTheme.apply( fontFeatures: const [FontFeature("ss01")], ); ``` ### Use Case: Variable Font Grade ```dart final theme = Theme.of(context).textTheme.apply( fontVariations: const [FontVariation('GRAD', 40.0)], ); ``` ## Testing Added 2 new tests to text_theme_test.dart: 1. `TextTheme apply fontFeatures fontVariations` - verifies all 15 styles receive the parameters 2. `TextTheme apply null fontFeatures fontVariations preserves original` - verifies null defaults don't overwrite existing values (fixes incomplete test from original PR) All 18 tests pass (including existing tests). ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ## Note This PR ports the original flutter/flutter PR #192730. The original PR should be closed with a reference to this new PR. The original PR had a test that claimed to verify both `fontFeatures` and `fontVariations` preservation, but only checked `fontFeatures`. This has been fixed in this ported version to properly verify both properties.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
fontFeaturesandfontVariationsparameters toTextTheme.apply(), passing them through to eachTextStyle.apply()call. This allows developers to apply font features and variations consistently across an entire text theme without modifying eachTextStyleindividually.Related Issue
Fixes #192692
Proposed Change
List<FontFeature>?andList<FontVariation>?parameters toTextTheme.apply()null, preserving backward compatibilityTextStyle.apply()calls withinTextTheme.apply()now passes these parameters throughTechnical Details
The
TextStyleclass already supportsfontFeaturesandfontVariationsin its ownapply()method (seetext_style.dart:995-996). TheTextTheme.apply()method was missing the ability to pass these through. This is a purely additive change — no existing behavior is modified.Use Case: Persian Digits
Use Case: Variable Font Grade
Testing
Added 2 new tests to
text_theme_test.dart:TextTheme apply fontFeatures fontVariations— verifies all 15 styles receive the parametersTextTheme apply null fontFeatures fontVariations preserves original— verifies null defaults don't overwrite existing valuesAll 18 tests pass (including existing tests).
Checklist