Unverified Commit 55eb5e11 authored by Jasper van Riet's avatar Jasper van Riet Committed by GitHub

Remove duplicate padding on CupertinoFormSection (#137039)

Fixes #128419, removes the duplicate padding that exists around the header and footer on `CupertinoFormSection`. This PR takes the implementation in #129065 and adds tests. I added the ones suggested in the PR review, as well as one more to test that `margin` is passed correctly from `CupertinoFormSection` to `CupertinoListSection`.

Note: I can't quite figure out if this is also a fix to #121543.

Potential review questions:

- Is the use of `offsetMoreOrLessEquals` here correct? I used it because of the vertical positioning. The horizontal positioning is always exact.

### Screenshots
<details>
  <summary>Before</summary>
  
![image](https://github.com/flutter/flutter/assets/5138348/1bce10c9-706a-40c8-a581-2dcb574ed937)
</details>
<details>
  <summary>After</summary>
  
![image](https://github.com/flutter/flutter/assets/5138348/cd1d529b-7be5-45df-af31-0f7760dc3fe9)
</details>
parent 06a66670
...@@ -11,12 +11,6 @@ import 'list_section.dart'; ...@@ -11,12 +11,6 @@ import 'list_section.dart';
// iOS 14.2 SDK. // iOS 14.2 SDK.
const EdgeInsetsDirectional _kFormDefaultInsetGroupedRowsMargin = EdgeInsetsDirectional.fromSTEB(20.0, 0.0, 20.0, 10.0); const EdgeInsetsDirectional _kFormDefaultInsetGroupedRowsMargin = EdgeInsetsDirectional.fromSTEB(20.0, 0.0, 20.0, 10.0);
// Standard header margin, determined from SwiftUI's Forms in iOS 14.2 SDK.
const EdgeInsetsDirectional _kFormDefaultHeaderMargin = EdgeInsetsDirectional.fromSTEB(20.0, 16.0, 20.0, 10.0);
// Standard footer margin, determined from SwiftUI's Forms in iOS 14.2 SDK.
const EdgeInsetsDirectional _kFormDefaultFooterMargin = EdgeInsetsDirectional.fromSTEB(20.0, 0.0, 20.0, 10.0);
/// An iOS-style form section. /// An iOS-style form section.
/// ///
/// The base constructor for [CupertinoFormSection] constructs an /// The base constructor for [CupertinoFormSection] constructs an
...@@ -205,10 +199,7 @@ class CupertinoFormSection extends StatelessWidget { ...@@ -205,10 +199,7 @@ class CupertinoFormSection extends StatelessWidget {
fontSize: 13.0, fontSize: 13.0,
color: CupertinoColors.secondaryLabel.resolveFrom(context), color: CupertinoColors.secondaryLabel.resolveFrom(context),
), ),
child: Padding( child: header!);
padding: _kFormDefaultHeaderMargin,
child: header,
));
final Widget? footerWidget = footer == null final Widget? footerWidget = footer == null
? null ? null
...@@ -217,10 +208,7 @@ class CupertinoFormSection extends StatelessWidget { ...@@ -217,10 +208,7 @@ class CupertinoFormSection extends StatelessWidget {
fontSize: 13.0, fontSize: 13.0,
color: CupertinoColors.secondaryLabel.resolveFrom(context), color: CupertinoColors.secondaryLabel.resolveFrom(context),
), ),
child: Padding( child: footer!);
padding: _kFormDefaultFooterMargin,
child: footer,
));
return _type == CupertinoListSectionType.base return _type == CupertinoListSectionType.base
? CupertinoListSection( ? CupertinoListSection(
......
...@@ -168,4 +168,57 @@ void main() { ...@@ -168,4 +168,57 @@ void main() {
final Iterable<RenderClipRRect> renderClips = tester.allRenderObjects.whereType<RenderClipRRect>(); final Iterable<RenderClipRRect> renderClips = tester.allRenderObjects.whereType<RenderClipRRect>();
expect(renderClips, isEmpty); expect(renderClips, isEmpty);
}); });
testWidgetsWithLeakTracking('Does not double up padding on header', (WidgetTester tester) async {
const Widget header = Text('Header');
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoFormSection(
header: header,
children: <Widget>[CupertinoTextFormFieldRow()],
),
),
),
);
expect(tester.getTopLeft(find.byWidget(header)), const Offset(20, 22));
});
testWidgetsWithLeakTracking('Does not double up padding on footer', (WidgetTester tester) async {
const Widget footer = Text('Footer');
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoFormSection(
footer: footer,
children: <Widget>[CupertinoTextFormFieldRow()],
),
),
),
);
expect(tester.getTopLeft(find.byWidget(footer)), offsetMoreOrLessEquals(const Offset(20, 65), epsilon: 1));
});
testWidgetsWithLeakTracking('Sets custom margin', (WidgetTester tester) async {
final Widget child = CupertinoTextFormFieldRow();
const double margin = 35;
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoFormSection(
margin: const EdgeInsets.all(margin),
children: <Widget>[child],
),
),
),
);
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(margin, 22 + margin), epsilon: 1));
});
} }
...@@ -223,4 +223,44 @@ void main() { ...@@ -223,4 +223,44 @@ void main() {
} }
} }
}); });
testWidgetsWithLeakTracking('does not show margin by default', (WidgetTester tester) async {
const Widget child = CupertinoListTile(title: Text('CupertinoListTile'));
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoListSection(
header: const Text('Header'),
children: const <Widget>[
child,
],
),
),
),
);
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(0, 41), epsilon: 1));
});
testWidgetsWithLeakTracking('shows custom margin', (WidgetTester tester) async {
const Widget child = CupertinoListTile(title: Text('CupertinoListTile'));
const double margin = 10;
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoListSection(
header: const Text('Header'),
margin: const EdgeInsets.all(margin),
children: const <Widget>[
child,
],
),
),
),
);
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(margin, 41 + margin), epsilon: 1));
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment