Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CupertinoFormSection header is wrapped by Padding twice, since Flutter 3.7.0 #128419

Closed
2 tasks done
yu1ro opened this issue Jun 7, 2023 · 10 comments · Fixed by #137039
Closed
2 tasks done

CupertinoFormSection header is wrapped by Padding twice, since Flutter 3.7.0 #128419

yu1ro opened this issue Jun 7, 2023 · 10 comments · Fixed by #137039
Labels
a: fidelity Matching the OEM platforms better c: regression It was better in the past than it is now f: cupertino flutter/packages/flutter/cupertino repository found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 framework flutter/packages/flutter repository. See also f: labels. good first issue Relatively approachable for first-time contributors has reproducible steps The issue has been confirmed reproducible and is ready to work on r: fixed Issue is closed as already fixed in a newer version team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@yu1ro
Copy link

yu1ro commented Jun 7, 2023

Is there an existing issue for this?

Steps to reproduce

  1. Use CupertinoFormSection.
  2. Use header parameter.
  3. Compare Flutter v3.3.10 and Flutter v3.7.0

Expected results

Actual results

Code sample

Code sample
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title),
      ),
      body: Column(
        children: [
          CupertinoFormSection(
            backgroundColor: Colors.transparent,
            header: const Text('This is section header'),
            children: const [
              ListTile(
                title: Text('This is ListTile title'),
              ),
            ],
          )
        ],
      ),
    );
  }
}

Screenshots or Video

Screenshots / Video demonstration

[Upload media here]

Logs

Logs
[Paste your logs here]

Flutter Doctor output

Doctor output
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.7.0, on macOS 13.4 22F66 darwin-arm64, locale en-JP)
[!] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
    ✗ Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/macos#android-setup for more details.
[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
[✓] Chrome - develop for the web
[!] Android Studio (not installed)
[✓] IntelliJ IDEA Ultimate Edition (version 2023.1.2)
[✓] VS Code (version 1.77.3)
[✓] Connected device (4 available)
[✓] HTTP Host Availability

! Doctor found issues in 2 categories.
@yu1ro
Copy link
Author

yu1ro commented Jun 7, 2023

First Padding is here.
Second Padding is here

@yu1ro yu1ro changed the title CupertinoFormSection header is wrapped Padding twice. since Flutter 3.7.0 CupertinoFormSection header is wrapped Padding twice, since Flutter 3.7.0 Jun 7, 2023
@yu1ro yu1ro changed the title CupertinoFormSection header is wrapped Padding twice, since Flutter 3.7.0 CupertinoFormSection header is wrapped by Padding twice, since Flutter 3.7.0 Jun 7, 2023
@huycozy huycozy added the in triage Presently being triaged by the triage team label Jun 8, 2023
@huycozy
Copy link
Member

huycozy commented Jun 8, 2023

Thanks for filing the issue. I also see the same behavior on the latest stable and master channels.

flutter doctor -v (stable and master)
[✓] Flutter (Channel stable, 3.10.4, on macOS 13.0.1 22A400 darwin-x64, locale en-VN)
    • Flutter version 3.10.4 on channel stable at /Users/huynq/Documents/GitHub/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 682aa387cf (2 days ago), 2023-06-05 18:04:56 -0500
    • Engine revision 2a3401c9bb
    • Dart version 3.0.3
    • DevTools version 2.23.1

[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-33, build-tools 32.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E222b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.78.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.66.0

[✓] Connected device (3 available)
    • Pixel 3a (mobile) • 964AY0WL20 • android-arm64  • Android 12 (API 32)
    • macOS (desktop)   • macos      • darwin-x64     • macOS 13.0.1 22A400 darwin-x64
    • Chrome (web)      • chrome     • web-javascript • Google Chrome 114.0.5735.106

[✓] Network resources
    • All expected network resources are available.

• No issues found!
[!] Flutter (Channel master, 3.12.0-2.0.pre.20, on macOS 13.0.1 22A400 darwin-x64, locale en-VN)
    • Flutter version 3.12.0-2.0.pre.20 on channel master at /Users/huynq/Documents/GitHub/flutter_master
    ! Warning: `flutter` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 1bd1b5014c (2 hours ago), 2023-06-07 18:16:30 -0700
    • Engine revision a5f7d5d75f
    • Dart version 3.1.0 (build 3.1.0-171.0.dev)
    • DevTools version 2.24.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-33, build-tools 32.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E222b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.78.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.66.0

[✓] Connected device (3 available)
    • Pixel 3a (mobile) • 964AY0WL20 • android-arm64  • Android 12 (API 32)
    • macOS (desktop)   • macos      • darwin-x64     • macOS 13.0.1 22A400 darwin-x64
    • Chrome (web)      • chrome     • web-javascript • Google Chrome 114.0.5735.106

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@huycozy huycozy added c: regression It was better in the past than it is now framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 and removed in triage Presently being triaged by the triage team labels Jun 8, 2023
@yu1ro
Copy link
Author

yu1ro commented Jun 8, 2023

Thanks for your triage.

@leighajarett leighajarett added a: fidelity Matching the OEM platforms better good first issue Relatively approachable for first-time contributors labels Jun 8, 2023
@jason-simmons
Copy link
Member

The layout of this widget changed in 0c40945a

@sompuradhruv
Copy link

hey i can help with this

@MitchellGoodwin
Copy link
Contributor

hey i can help with this

@sompuradhruv If you want to take a crack at this, feel free! Tag me in anything that needs a look.

@MitchellGoodwin
Copy link
Contributor

There's the related issue #121543. With the two issues combined, removing the padding around ListSection seems like the way to go, as if padding is needed by the developer, they can simply wrap the widget with padding themselves.

It'd be a simple change, but could possibly break some tests getting merged in. But it'd still be better to do it now, rather than later.

@sompuradhruv
Copy link

Hey should I remove the padding section inside the form_selction.dart file highlighted in line 208? will that be enough

@arrrtem22
Copy link

@sompuradhruv already done in #129048

fluttermirroringbot pushed a commit that referenced this issue Nov 2, 2023
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>
@huycozy huycozy added the r: fixed Issue is closed as already fixed in a newer version label Nov 3, 2023
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: fidelity Matching the OEM platforms better c: regression It was better in the past than it is now f: cupertino flutter/packages/flutter/cupertino repository found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 framework flutter/packages/flutter repository. See also f: labels. good first issue Relatively approachable for first-time contributors has reproducible steps The issue has been confirmed reproducible and is ready to work on r: fixed Issue is closed as already fixed in a newer version team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants