Update layout tutorial for current best practices #9893

atsansone · 2023-12-04T02:02:08Z

This refactors the Layout Tutorial for current best practice including adding screenshots and using Padding where needed.

Fixes #2108
Fixes #1799
Fixes #9118

flutter-website-bot · 2023-12-04T02:11:38Z

Visit the preview URL for this PR (updated for commit 2e2eeb1):

https://flutter-docs-prod--pr9893-fix-2108-1db4ypog.web.app

domesticmouse · 2023-12-04T07:29:53Z

Test failure?

atsansone · 2023-12-06T20:25:01Z

The local run of the errored test went fine. @domesticmouse : Any hint as to why this went awry?

parlough · 2023-12-06T20:40:17Z

You'll need to update your Flutter version to todays release to see the error. There was a bug in the analyzer in earlier Dart 3.2 versions that allowed const when it shouldn't have. I have a fix in #9901

gspencergoog · 2023-12-07T19:58:00Z

examples/layout/base/lib/main.dart

@@ -1,4 +1,4 @@
-// Copyright 2021 The Flutter team. All rights reserved.
+// Copyright 2021-2023 The Flutter team. All rights reserved.


In the Flutter repo at least, we don't typically update the copyright dates. It's sufficient to include just the initial date.

+1 for initial date only

gspencergoog · 2023-12-07T20:30:04Z

examples/layout/lakes/interactive/lib/main.dart

-        Container(
-          margin: const EdgeInsets.only(top: 8),
+        Padding(
+          padding: const EdgeInsets.only(top: 8),


Padding is different from margin, but that may be just fine. Padding is on the inside of the shape, and margin is on the outside. If the container doesn't have a color or decoration, then it really doesn't matter much.

gspencergoog · 2023-12-07T20:33:25Z

examples/layout/lakes/step4/lib/main.dart

+      child: Row(
+        mainAxisAlignment: MainAxisAlignment.spaceEvenly,
+        children: [
+          _buildButtonColumn(color, Icons.call, 'CALL'),


You could probably also convert _buildButtonColumn into a separate StatelessWidget pretty easily. It only has those three parameters.

I like that idea. I just did it.

gspencergoog

domesticmouse · 2023-12-08T03:39:43Z

examples/layout/lakes/step4/lib/main.dart

+    );
+  }
+}
+// #enddocregion TextAdd


It looks like you have duplicate end doc region tags for TextAdd on line 11 and line 47. I have no idea how this interacts with the excerpter.

It creates a // ... break between the first #enddocregion and the second #docregion.

domesticmouse · 2023-12-08T04:22:32Z

Also:

warning • Unused import: 'package:flutter/foundation.dart' • test/widget_test.dart:4:8 • unused_import

ericwindmill

I only left "food for thought" / non-blocking comments. LGTM

ericwindmill · 2023-12-11T17:21:39Z

src/ui/layout/tutorial.md

+   }
+   ```
+
+## Diagram the layout


While I do prefer this updated version, I've always found this section of the page to be superfluous. Are readers really going to break out a pad of paper and draw a layout?

I can see an argument for why it's important, but I also think the truly valuable information here is in the annotated images, which could just be integrated into the steps themselves (starting with ## Add the title screen widget

Yeah, I see that point and debated removing it myself, but getting the developer to stop and think about design before coding didn't see like a bad idea.

ericwindmill · 2023-12-11T17:30:19Z

src/ui/layout/tutorial.md

-    painted red, and the text "41". The entire row is in
-    a `Container` and padded along each edge by 32 pixels.
-    Add the title section to the app body like this:
+    {:.numbered-code-notes}


@parlough Does this functionality exist on the Dart site? How easy is it to update with custom syntax in 11ty? Given that we're building a bigger tutorial (i.e. First Week Experience), I think this functionality is quite important and it'd be nice to consider whether we can (and should) "enhance it" with better syntax highlighting or some other way to annotate the code.

@ericwindmill : You have no idea how often I want to add line numbers. In previous lifetimes, I would add them to anything longer than 10 lines.

We don't have any standard functionality for this, but we could always implement something if people feel they will benefit from it. The main concern is accessibility and the ability for understanding to carry over to the explanations, so it would require a review of similar functionality out there today and any related accessibility concerns.

Generally (but not always) if a sample is so big that it needs multiple numbered explanations, it's best split up.

I'll do some research and keep this in mind as a use case to support!

This reverts commit d93a46c.

parlough · 2023-12-14T03:50:04Z

Note I reverted this in #9927 to fix a few issues.

There are potentially other small issues, but at least:

"Adding iOS app extensions" screenshots are distorted #9925 caused by adding width: 100% to site-figure-container. Likely should be max-width.
The code block in the Update the app to display the button section section failed to render.
Step 4 of the Interactivity page was changed so the diff no longer makes sense.

Please reopen and I'll complete a review. Thanks so much!

atsansone added the review.tech Awaiting Technical Review label Dec 4, 2023

atsansone requested review from domesticmouse, johnpryan and khanhnwin as code owners December 4, 2023 02:02

atsansone assigned gspencergoog Dec 4, 2023

atsansone requested review from parlough, MaryaBelanger and sfshaza2 as code owners December 4, 2023 02:02

atsansone requested a review from gspencergoog December 4, 2023 02:02

parlough requested a review from ericwindmill December 4, 2023 04:18

gspencergoog reviewed Dec 7, 2023

View reviewed changes

gspencergoog approved these changes Dec 7, 2023

View reviewed changes

domesticmouse reviewed Dec 8, 2023

View reviewed changes

domesticmouse approved these changes Dec 8, 2023

View reviewed changes

parlough assigned ericwindmill and sfshaza2 and unassigned gspencergoog Dec 10, 2023

parlough added the review.copy Awaiting Copy Review label Dec 10, 2023

atsansone unassigned ericwindmill and sfshaza2 Dec 11, 2023

atsansone removed the request for review from ericwindmill December 11, 2023 05:00

atsansone removed the review.tech Awaiting Technical Review label Dec 11, 2023

ericwindmill approved these changes Dec 11, 2023

View reviewed changes

atsansone added 2 commits December 12, 2023 15:43

Updating per Issue 2108

4cad9fa

Removed the alignment of widgets page as it's not live and raises errors

e96217f

atsansone added 7 commits December 12, 2023 15:43

Restore index file

4d7c0b4

Updated code excerpts

1c632a5

Updates per @gspencergoog review

cd522b5

Updated per @domesticmouse review

9a7b99d

Updated code examples

c9ea5b2

Fixed auto-linting changes

97ff281

Fixed code-excerpts

a5bdc02

atsansone force-pushed the fix-2108 branch from 7f83328 to a5bdc02 Compare December 12, 2023 21:45

parlough assigned sfshaza2 Dec 12, 2023

Merge branch 'main' into fix-2108

2e2eeb1

atsansone merged commit d93a46c into flutter:main Dec 13, 2023
9 checks passed

atsansone unassigned sfshaza2 Dec 13, 2023

atsansone added st.RFM Ready to merge or land and removed review.copy Awaiting Copy Review labels Dec 13, 2023

parlough added a commit that referenced this pull request Dec 14, 2023

Revert "Update layout tutorial for current best practices (#9893)"

a4792b7

This reverts commit d93a46c.

parlough mentioned this pull request Dec 14, 2023

Revert "Update layout tutorial for current best practices" #9927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update layout tutorial for current best practices #9893

Update layout tutorial for current best practices #9893

atsansone commented Dec 4, 2023 •

edited

Loading

flutter-website-bot commented Dec 4, 2023 •

edited

Loading

domesticmouse commented Dec 4, 2023

atsansone commented Dec 6, 2023

parlough commented Dec 6, 2023

gspencergoog Dec 7, 2023

domesticmouse Dec 7, 2023

gspencergoog Dec 7, 2023

gspencergoog Dec 7, 2023

atsansone Dec 7, 2023

gspencergoog left a comment

domesticmouse Dec 8, 2023

atsansone Dec 8, 2023

domesticmouse commented Dec 8, 2023

ericwindmill left a comment

ericwindmill Dec 11, 2023

atsansone Dec 11, 2023

ericwindmill Dec 11, 2023

atsansone Dec 11, 2023

parlough Dec 11, 2023

parlough commented Dec 14, 2023

		@@ -1,4 +1,4 @@
		// Copyright 2021 The Flutter team. All rights reserved.
		// Copyright 2021-2023 The Flutter team. All rights reserved.

Update layout tutorial for current best practices #9893

Update layout tutorial for current best practices #9893

Conversation

atsansone commented Dec 4, 2023 • edited Loading

flutter-website-bot commented Dec 4, 2023 • edited Loading

domesticmouse commented Dec 4, 2023

atsansone commented Dec 6, 2023

parlough commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gspencergoog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domesticmouse commented Dec 8, 2023

ericwindmill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parlough commented Dec 14, 2023

atsansone commented Dec 4, 2023 •

edited

Loading

flutter-website-bot commented Dec 4, 2023 •

edited

Loading