Skip to content

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Jul 24, 2023

Fixes #7037

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 24, 2023
@Hixie
Copy link
Contributor Author

Hixie commented Jul 24, 2023

cc @justinmc - you might want to use this around the labels in a text field to prevent buttons in the same row from aligning with the text field label rather than the text field contents

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits 👍

Is this what you mean about aligning with the content instead of the label? Looks like it works now but I'm probably misunderstanding, or maybe we're already disregarding labels etc. in baseline calculations.

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

// See https://github.com/flutter/flutter/pull/131220

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(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

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

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final TextEditingController _controller = TextEditingController(
    //text: 'Hello text field.',
  );

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Row(
              crossAxisAlignment: CrossAxisAlignment.baseline,
              textBaseline: TextBaseline.alphabetic,
              textDirection: TextDirection.ltr,
              children: [
                ConstrainedBox(
                  constraints: BoxConstraints.loose(const Size(150.0, double.infinity)),
                  child: TextField(
                    controller: _controller,
                    decoration: const InputDecoration(
                      labelText: 'label',
                      labelStyle: TextStyle(fontSize: 64.0),
                    )
                  ),
                ),
                const Text(
                  'a',
                  textDirection: TextDirection.ltr,
                  style: TextStyle(fontSize: 128.0, fontFamily: 'FlutterTest'), // places baseline at y=96
                ),
                const Text(
                  'b',
                  textDirection: TextDirection.ltr,
                  style: TextStyle(fontSize: 32.0, fontFamily: 'FlutterTest'), // 24 above baseline, 8 below baseline
                ),
              ],
            ),
          ],
        ),
      ),
    );
  }
}

Screenshot from 2023-07-25 10-42-16
Screenshot from 2023-07-25 10-42-00

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be super.child?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately not, it's a positional argument in the superclass

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed that it was positional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Too many newlines here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a section separator (the next section starts with an all-caps heading), the rest of the file (with one exception i can fix) has the same double blank line before headings.

I don't feel strongly one way or the other if you would prefer I removed the double blank lines everywhere in this file before the headings, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say don't worry about it here, maybe if it comes up again we can make this file all 1 line.

@Hixie
Copy link
Contributor Author

Hixie commented Jul 26, 2023

Is this what you mean about aligning with the content instead of the label? Looks like it works now but I'm probably misunderstanding, or maybe we're already disregarding labels etc. in baseline calculations.

It was what I meant. I wonder how we make it work today! It used to be broken (see comments in #7037).

@justinmc
Copy link
Contributor

I'll post in the issue about the button-textfield alignment thing.

@Hixie Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2023
@auto-submit auto-submit bot merged commit 48f08e3 into flutter:master Jul 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 27, 2023
flutter/flutter@61fd11d...dd9764e

2023-07-27 [email protected] Proposal to add barrier configs for showDatePicker, showTimePicker and showAboutDialog. (flutter/flutter#131306)
2023-07-27 [email protected] Fix ios_add2app Podfile (flutter/flutter#131263)
2023-07-27 [email protected] Add DeviceLab build+test separation doc (flutter/flutter#131365)
2023-07-27 [email protected] IgnoreBaseline widget (flutter/flutter#131220)
2023-07-27 [email protected] Add 'vm:keep-name' pragmas to platform channel classes (flutter/flutter#131271)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
vashworth pushed a commit to vashworth/flutter that referenced this pull request Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Ignore for baseline purposes" widget

2 participants