Skip to content

Commit 55eb5e1

Browse files
Remove duplicate padding on CupertinoFormSection (flutter#137039)
Fixes flutter#128419, removes the duplicate padding that exists around the header and footer on `CupertinoFormSection`. This PR takes the implementation in flutter#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 flutter#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>
1 parent 06a6667 commit 55eb5e1

File tree

3 files changed

+95
-14
lines changed

3 files changed

+95
-14
lines changed

packages/flutter/lib/src/cupertino/form_section.dart

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@ import 'list_section.dart';
1111
// iOS 14.2 SDK.
1212
const EdgeInsetsDirectional _kFormDefaultInsetGroupedRowsMargin = EdgeInsetsDirectional.fromSTEB(20.0, 0.0, 20.0, 10.0);
1313

14-
// Standard header margin, determined from SwiftUI's Forms in iOS 14.2 SDK.
15-
const EdgeInsetsDirectional _kFormDefaultHeaderMargin = EdgeInsetsDirectional.fromSTEB(20.0, 16.0, 20.0, 10.0);
16-
17-
// Standard footer margin, determined from SwiftUI's Forms in iOS 14.2 SDK.
18-
const EdgeInsetsDirectional _kFormDefaultFooterMargin = EdgeInsetsDirectional.fromSTEB(20.0, 0.0, 20.0, 10.0);
19-
2014
/// An iOS-style form section.
2115
///
2216
/// The base constructor for [CupertinoFormSection] constructs an
@@ -205,10 +199,7 @@ class CupertinoFormSection extends StatelessWidget {
205199
fontSize: 13.0,
206200
color: CupertinoColors.secondaryLabel.resolveFrom(context),
207201
),
208-
child: Padding(
209-
padding: _kFormDefaultHeaderMargin,
210-
child: header,
211-
));
202+
child: header!);
212203

213204
final Widget? footerWidget = footer == null
214205
? null
@@ -217,10 +208,7 @@ class CupertinoFormSection extends StatelessWidget {
217208
fontSize: 13.0,
218209
color: CupertinoColors.secondaryLabel.resolveFrom(context),
219210
),
220-
child: Padding(
221-
padding: _kFormDefaultFooterMargin,
222-
child: footer,
223-
));
211+
child: footer!);
224212

225213
return _type == CupertinoListSectionType.base
226214
? CupertinoListSection(

packages/flutter/test/cupertino/form_section_test.dart

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,57 @@ void main() {
168168
final Iterable<RenderClipRRect> renderClips = tester.allRenderObjects.whereType<RenderClipRRect>();
169169
expect(renderClips, isEmpty);
170170
});
171+
172+
testWidgetsWithLeakTracking('Does not double up padding on header', (WidgetTester tester) async {
173+
const Widget header = Text('Header');
174+
175+
await tester.pumpWidget(
176+
CupertinoApp(
177+
home: Center(
178+
child: CupertinoFormSection(
179+
header: header,
180+
children: <Widget>[CupertinoTextFormFieldRow()],
181+
),
182+
),
183+
),
184+
);
185+
186+
expect(tester.getTopLeft(find.byWidget(header)), const Offset(20, 22));
187+
});
188+
189+
testWidgetsWithLeakTracking('Does not double up padding on footer', (WidgetTester tester) async {
190+
const Widget footer = Text('Footer');
191+
192+
await tester.pumpWidget(
193+
CupertinoApp(
194+
home: Center(
195+
child: CupertinoFormSection(
196+
footer: footer,
197+
children: <Widget>[CupertinoTextFormFieldRow()],
198+
),
199+
),
200+
),
201+
);
202+
203+
expect(tester.getTopLeft(find.byWidget(footer)), offsetMoreOrLessEquals(const Offset(20, 65), epsilon: 1));
204+
});
205+
206+
testWidgetsWithLeakTracking('Sets custom margin', (WidgetTester tester) async {
207+
final Widget child = CupertinoTextFormFieldRow();
208+
209+
const double margin = 35;
210+
211+
await tester.pumpWidget(
212+
CupertinoApp(
213+
home: Center(
214+
child: CupertinoFormSection(
215+
margin: const EdgeInsets.all(margin),
216+
children: <Widget>[child],
217+
),
218+
),
219+
),
220+
);
221+
222+
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(margin, 22 + margin), epsilon: 1));
223+
});
171224
}

packages/flutter/test/cupertino/list_section_test.dart

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,4 +223,44 @@ void main() {
223223
}
224224
}
225225
});
226+
227+
testWidgetsWithLeakTracking('does not show margin by default', (WidgetTester tester) async {
228+
const Widget child = CupertinoListTile(title: Text('CupertinoListTile'));
229+
230+
await tester.pumpWidget(
231+
CupertinoApp(
232+
home: Center(
233+
child: CupertinoListSection(
234+
header: const Text('Header'),
235+
children: const <Widget>[
236+
child,
237+
],
238+
),
239+
),
240+
),
241+
);
242+
243+
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(0, 41), epsilon: 1));
244+
});
245+
246+
testWidgetsWithLeakTracking('shows custom margin', (WidgetTester tester) async {
247+
const Widget child = CupertinoListTile(title: Text('CupertinoListTile'));
248+
const double margin = 10;
249+
250+
await tester.pumpWidget(
251+
CupertinoApp(
252+
home: Center(
253+
child: CupertinoListSection(
254+
header: const Text('Header'),
255+
margin: const EdgeInsets.all(margin),
256+
children: const <Widget>[
257+
child,
258+
],
259+
),
260+
),
261+
),
262+
);
263+
264+
expect(tester.getTopLeft(find.byWidget(child)), offsetMoreOrLessEquals(const Offset(margin, 41 + margin), epsilon: 1));
265+
});
226266
}

0 commit comments

Comments
 (0)