Skip to content

Conversation

@bleroux
Copy link
Contributor

@bleroux bleroux commented Dec 17, 2025

Description

This PR makes MaterialBannerTheme.data non nullable similarly to the data field of other themes.
(See #179736 (comment) for context).

Tests

Adds 2 tests.
Those tests were missing tests for MaterialBannerTheme.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Dec 17, 2025
Copy link
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 correctly makes MaterialBannerTheme.data non-nullable, aligning it with other theme data patterns in the framework. The accompanying tests are well-written and cover the new functionality of using a local MaterialBannerTheme. I've added one comment regarding code duplication in the new tests, which could be refactored for better maintainability.

@bleroux bleroux requested a review from QuncCccccc December 17, 2025 18:48
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

I don't think we need to remove the nullable type if it already exists because this may cause breakages.

@bleroux
Copy link
Contributor Author

bleroux commented Dec 17, 2025

As you want, the motivation were:

  • this is the only theme class with a nullable data field.
  • this was not done on purpose (it was part of large null safety migration PR).
  • if some existing code relied on this behavior, it meant that the MaterialBannerTheme had no effect in those cases and could be removed.

@bleroux bleroux closed this Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants