Repository navigation
Conversation
github-actions
Bot
added
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies SearchAnchor.bar to allow the internal SearchBar to inherit padding from SearchBarThemeData by removing a hardcoded default value. New tests were added to verify that theme padding is correctly applied and can be overridden by the barPadding property. Review feedback suggests using const for WidgetStatePropertyAll instances in the tests for performance and consistency, and adding comments to explain assertions that expect multiple widgets to improve maintainability.
|
Pushed |
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
Thanks a lot for your contribution! Seems the change doesn't really fix the issue, and we have another PR #186037 fixing the same issue, so I'm closing this PR for now:)
| padding: | ||
| barPadding ?? | ||
| const MaterialStatePropertyAll<EdgeInsets>(EdgeInsets.symmetric(horizontal: 16.0)), | ||
| padding: barPadding, |
There was a problem hiding this comment.
This seems not a correct fix since the theme padding is still not respected and we removed the default value.
| (Widget widget) => widget is Padding && widget.padding == themePadding, | ||
| ), | ||
| ), | ||
| findsNWidgets(searchBarsForAnchorAndView), |
There was a problem hiding this comment.
This test should check whether themePaddingProperty is respected.
Part of #185889.
SearchAnchor.barwas passing its own hard-coded default padding to the innerSearchBarwheneverbarPaddingwas null. That preventedSearchBarfrom falling back toSearchBarThemeData.padding.This removes that hard-coded
barPaddingfallback so the normalSearchBarresolution order applies:barPaddingSearchBarThemeData.paddingRegression tests cover both themed padding and explicit
barPaddingoverriding the theme.Note:
SearchBarThemeDatadoes not currently expose aleadingproperty, so this PR addresses the padding behavior from the issue's repro.Validation:
dart analyze packages/flutter/lib/src/material/search_anchor.dart packages/flutter/test/material/search_anchor_test.dartflutter test packages/flutter/test/material/search_anchor_test.dart