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.insetGrouped divider margin issue #119895

Open
jwseph opened this issue Feb 3, 2023 · 6 comments
Open

CupertinoFormSection.insetGrouped divider margin issue #119895

jwseph opened this issue Feb 3, 2023 · 6 comments
Labels
f: cupertino flutter/packages/flutter/cupertino repository found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@jwseph
Copy link

jwseph commented Feb 3, 2023

Steps to Reproduce

  1. Use a CupertinoFormSection.insetGrouped with at least two CupertinoFormRow children

Expected result:
Since CupertinoFormSection's dividers' leading margins cannot be customized (unlike CupertinoListSection), its dividers' leading margins should align with the text.

Actual result:
The dividers' leading margins correctly align with the text for the base CupertinoFormSection, but they do not align for CupertinoFormSection.insetGrouped. Tested in Android and DartPad

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

void main() => runApp(const TestApp());

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

  @override
  Widget build(BuildContext context) {
    return CupertinoApp(
      theme: const CupertinoThemeData(
        brightness: Brightness.light,
      ),
      home: Container(
        color: CupertinoColors.systemGroupedBackground.color,
        child: SafeArea(
          child: Column(
            children: [
              CupertinoFormSection(
                header: const Text('CupertinoFormSection (not .insetGrouped, working)'),
                children: [
                  for (int i = 0; i < 4; i++) CupertinoTextFormFieldRow(
                    prefix: const Padding(
                      padding: EdgeInsets.only(right: 16),
                      child: Text('Prefix'),
                    ),
                    placeholder: 'Placeholder',
                  ),
                ],
              ),
              const SizedBox(height: 24),
              CupertinoFormSection.insetGrouped(
                header: const Text('CupertinoFormSection.insetGrouped, not working'),
                children: [
                  for (int i = 0; i < 4; i++) CupertinoTextFormFieldRow(
                    prefix: const Padding(
                      padding: EdgeInsets.only(right: 16),
                      child: Text('Prefix'),
                    ),
                    placeholder: 'Placeholder',
                  ),
                ],
              ),
            ],
          ),
        ),
      ),
    );
  }
}
Image

screenshot

Logs
[√] Flutter (Channel stable, 3.7.0, on Microsoft Windows [Version 10.0.19045.2486], locale en-US)
    • Flutter version 3.7.0 on channel stable at C:\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision b06b8b2710 (10 days ago), 2023-01-23 16:55:55 -0800
    • Engine revision b24591ed32
    • Dart version 2.19.0
    • DevTools version 2.20.1

[√] Windows Version (Installed version of Windows is version 10 or higher)

[√] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at C:\Users\Jet_g\AppData\Local\Android\sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Build Tools 2019 16.11.2)
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools
    • Visual Studio Build Tools 2019 version 16.11.31624.102
    • Windows 10 SDK version 10.0.19041.0

[√] Android Studio (version 2021.2)
    • Android Studio at C:\Program Files\Android\Android Studio
    • 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 11.0.12+7-b1504.28-7817840)

[√] VS Code, 64-bit edition (version 1.73.1)
    • VS Code at C:\Program Files\Microsoft VS Code
    • Flutter extension version 3.58.0

[√] Connected device (4 available)
    • SM A136U1 (mobile) • R5CT12A42CV • android-arm    • Android 12 (API 31)
    • Windows (desktop)  • windows     • windows-x64    • Microsoft Windows [Version 10.0.19045.2486]
    • Chrome (web)       • chrome      • web-javascript • Google Chrome 108.0.5359.125
    • Edge (web)         • edge        • web-javascript • Microsoft Edge 109.0.1518.61

[√] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Feb 3, 2023
@darshankawar
Copy link
Member

Thanks for the report @jwseph
According to CupertinoFormSection.insetGrouped documentation, it creates a round-edged and padded section that is commonly seen in notched-displays like iPhone X and beyond.

While, just using CupertinoFormSection, it says The base constructor for CupertinoFormSection constructs an edge-to-edge style section which includes an iOS-style header, rows, the dividers between rows, and borders on top and bottom of the rows.

So this is WAI per documentation. What do you think ?

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Feb 3, 2023
@znromonk
Copy link

znromonk commented Feb 3, 2023

@darshankawar I think OP is pointing towards the leading padding of the divider for insetGrouped not being aligned with text.

@jwseph
Copy link
Author

jwseph commented Feb 3, 2023

Thanks @znromonk, that is what I meant. I'll edit the issue for clarification.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Feb 3, 2023
@znromonk
Copy link

znromonk commented Feb 3, 2023

@jwseph I think I have figured out the reason for the inconsistency.

CupertinoFormSection.insetGrouped uses CupertinoListSection.insetGrouped with hasLeading: false.

Referring to the example in the comment from the PR that merged the CupertinoListSection, for a CupertinoListSection.insetGrouped widget the CupertinoListTile.notched child provides the correct padding to align the text and the divider.

CupertinoListTile.notched in turn has a left padding of 28.0

const EdgeInsetsDirectional _kNotchedPaddingWithoutLeading = EdgeInsetsDirectional.fromSTEB(28.0, 10.0, 14.0, 10.0);

case _CupertinoListTileType.notched:
padding = widget.leading == null ? _kNotchedPaddingWithoutLeading : _kNotchedPadding;
break;
}

The reason the divider doesn't line up with the text for CupertinoFormSection.insetGrouped's children (CupertinoTextFormFieldRow) is because they have a default left padding of 20.0 (from CupertinoFormRow)

const EdgeInsetsGeometry _kDefaultPadding =
EdgeInsetsDirectional.fromSTEB(20.0, 6.0, 6.0, 6.0);

return Padding(
padding: padding ?? _kDefaultPadding,

Workaround

The workaround is to pass padding to the CupertinoTextFormFieldRow but with left padding of 28.0.

I feel this would need to an official fix. Something like CupertinoTextFormFieldRow.notched similar to CupertinoListTile.notched.

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

void main() => runApp(const TestApp());

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

  @override
  Widget build(BuildContext context) {
    return CupertinoApp(
      theme: const CupertinoThemeData(
        brightness: Brightness.light,
      ),
      home: Container(
        color: CupertinoColors.systemGroupedBackground.color,
        child: SafeArea(
          child: SingleChildScrollView(
            child: Column(
              children: [
                CupertinoFormSection(
                  header: const Text(
                      'CupertinoFormSection (not .insetGrouped, working)'),
                  children: [
                    for (int i = 0; i < 4; i++)
                      CupertinoTextFormFieldRow(
                        prefix: const Padding(
                          padding: EdgeInsets.only(right: 16),
                          child: Text('Prefix'),
                        ),
                        placeholder: 'Placeholder',
                      ),
                  ],
                ),
                const SizedBox(height: 24),
                CupertinoFormSection.insetGrouped(
                  header: const Text(
                      'CupertinoFormSection.insetGrouped, not working'),
                  children: [
                    for (int i = 0; i < 4; i++)
                      CupertinoTextFormFieldRow(
                        prefix: const Padding(
                          padding: EdgeInsets.only(right: 16),
                          child: Text('Prefix'),
                        ),
                        placeholder: 'Placeholder',
                      ),
                  ],
                ),
                const SizedBox(height: 24),
                CupertinoFormSection.insetGrouped(
                  header:
                      const Text('CupertinoFormSection.insetGrouped, working'),
                  children: [
                    for (int i = 0; i < 4; i++)
                      CupertinoTextFormFieldRow(
                        padding: const EdgeInsetsDirectional.fromSTEB(
                            28.0, 6.0, 6.0, 6.0),
                        prefix: const Padding(
                          padding: EdgeInsets.only(right: 16),
                          child: Text('Prefix'),
                        ),
                        placeholder: 'Placeholder',
                      ),
                  ],
                ),
              ],
            ),
          ),
        ),
      ),
    );
  }
}
Screenshot image

@jwseph
Copy link
Author

jwseph commented Feb 4, 2023

@znromonk Ok, that makes sense. I had thought the divider margin and not the form row padding was the issue.

@darshankawar
Copy link
Member

Thanks for the analysis and sharing update @znromonk

stable, master flutter doctor -v
[!] Flutter (Channel stable, 3.7.0, on macOS 12.2.1 21D62 darwin-x64, locale
    en-GB)
    • Flutter version 3.7.0 on channel stable at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision b06b8b2710 (28 hours ago), 2023-01-23 16:55:55 -0800
    • Engine revision b24591ed32
    • Dart version 2.19.0
    • DevTools version 2.20.1
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

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

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

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

[!] Flutter (Channel master, 3.8.0-4.0.pre, on macOS 12.2.1 21D62 darwin-x64,
    locale en-GB)
    • Flutter version 3.8.0-4.0.pre on channel master at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision b8f5394a5c (22 hours ago), 2023-02-04 23:06:19 -0800
    • Engine revision 2a104cdfcd
    • Dart version 3.0.0 (build 3.0.0-204.0.dev)
    • DevTools version 2.21.1
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

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

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

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.



@darshankawar darshankawar added 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.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 and removed in triage Presently being triaged by the triage team labels Feb 6, 2023
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-design Owned by Design Languages team triaged-design Triaged by Design Languages team labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: cupertino flutter/packages/flutter/cupertino repository found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
None yet
Development

No branches or pull requests

4 participants