Repository navigation
Conversation
github-actions
Bot
added
triage-framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Companion to flutter/flutter#185152, which adds a `hyphens` parameter to `TextStyle.getParagraphStyle`. The `_TextStyleProxy` test double in material_ui overrides that method and must accept the new optional named parameter to remain a valid override once the framework change rolls in. As an optional named parameter it is a valid override against both the current pinned Flutter and the updated one, so this can land independently.
1c732c8 to
6726db5
Compare
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Thanks so much for proactively opening this PR! We're still getting going with material_ui but I think this is exactly what we need.
This PR should not merge until the SDK is bumped to include flutter/flutter#185152. Unless that PR is cherry picked, that likely won't be until after 3.47.
In fact, it may need to be merged into a feature branch... CC @chunhtai do we have a process in place for bumping the SDK version in other packages?
I believe this PR will not pass CI until the SDK version is bumped, so I'll go ahead and LGTM it and add the CI label under the assumption it will fail.
|
We should close this for now. This package is not yet accepting contributions, and we want to avoid confusing other contributors. @justinmc is correct as well, we won't be able to make this update here until the change in the core SDK rolls to stable. After which this package will be updated to stable, at which point this change can be made. |
|
Thanks @Piinks, that makes sense — I hadn't accounted for material_ui not accepting contributions yet, and you and @justinmc are right that the override can't reference |
|
There isn't currently a process besides maintaining a feature branch for sdk bump. |
… when framework `TextStyle` changes (#12728) The mechanism was introduced in flutter/flutter#12552 as a means to notify `TextTheme` maintainers when new fields are added to`TextStyle`. In the past since they both live in the `flutter/flutter` repo, they can and should be updated in the same PR. But now that `TextStyle` lives in an upstream repo, this test would make any new feature from being added to `TextStyle` a breaking change, which is very undesirable. Making the change would mean `material_ui` maintainers won't be notified via failing tests when the upstream `TextStyle` gets a new field on master (If I understand correctly, `material_ui` is analyzed/tested against `flutter/flutter`'s master branch). But that's already the case for any other new features added to the framework. This is for flutter/flutter#185152. Previous attempt: #12158. ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. [^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.
4 participants
Adds the new
hyphensparameter to the_TextStyleProxy.getParagraphStyleoverride inmaterial_ui'stheme_test.dart.This is a companion to flutter/flutter#185152, which adds an optional
hyphensparameter toTextStyle.getParagraphStyleas part of the soft-hyphen (U+00AD) rendering feature._TextStyleProxyimplements TextStyleand overridesgetParagraphStyle; once #185152 rolls into this repo's pinned Flutter the override is invalid unless it also accepts the new parameter. This is theflutter_pluginspresubmit failure surfaced on #185152.Because
hyphensis added as an optional named parameter, the override stays valid against both the current pinned Flutter (where it is simply an extra optional param) and the updated framework (where it matches the new supertype signature). So this PR is green on its own and unblocks #185152'sflutter_pluginscheck once merged.Fixes flutter/flutter#18443 (companion to flutter/flutter#185152).
Pre-Review Checklist
[shared_preferences]///).CHANGELOG/version exemption: test-only change — this updates a test double (
_TextStyleProxy) whose overrides allthrow UnimplementedError(); there is no shipped code change, so no version bump or CHANGELOG entry is warranted.Test exemption: mechanical override-signature update to a test double; the added parameter is unused (
throw UnimplementedError()), so there is no behavior to test.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2