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

Container widget should wrap its content; not fill its parent #4949

Closed
mehmetf opened this issue Jul 18, 2016 · 14 comments
Closed

Container widget should wrap its content; not fill its parent #4949

mehmetf opened this issue Jul 18, 2016 · 14 comments
Labels
framework flutter/packages/flutter repository. See also f: labels.

Comments

@mehmetf
Copy link
Contributor

mehmetf commented Jul 18, 2016

Using the material container widget to draw a decoration (border) for a child widget. A side effect of this is that the border is not just under the child widget but extends to fill whatever space is left on the parent.

IMO, container should respect the bounds of the child widgets or at least provide an additional ctr param to tell it to wrap its child.

Current outcome:

image

Desired outcome:

image

Workaround is to wrap the container in a Row() widget.

@eseidelGoogle eseidelGoogle added framework flutter/packages/flutter repository. See also f: labels. customer: leafy labels Jul 18, 2016
@eseidelGoogle
Copy link
Contributor

@Hixie

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

Do you have some example code that shows the issue? I tried reproducing this issue in the Stocks example, and it behaved the way I would expect:

diff --git a/examples/stocks/lib/stock_home.dart b/examples/stocks/lib/stock_home.dart
index ea5c9c1..8957fa3 100644
--- a/examples/stocks/lib/stock_home.dart
+++ b/examples/stocks/lib/stock_home.dart
@@ -211,7 +211,17 @@ class StockHomeState extends State<StockHome> {
   Widget buildAppBar() {
     return new AppBar(
       elevation: 0,
-      title: new Text(StockStrings.of(context).title()),
+      // title: new Text(StockStrings.of(context).title()),
+      title: new Align(alignment: FractionalOffset.centerLeft, child: new DropDownButton<int>(
+        items: <DropDownMenuItem<int>>[
+          new DropDownMenuItem<int>(value: 1, child: new Text("One")),
+          new DropDownMenuItem<int>(value: 2, child: new Text("Two")),
+          new DropDownMenuItem<int>(value: 3, child: new Text("Three")),
+          new DropDownMenuItem<int>(value: 4, child: new Text("Four")),
+        ],
+        value: 2,
+        onChanged: (_) { }
+      )),
       actions: <Widget>[
         new IconButton(
           icon: new Icon(Icons.search),

@mehmetf
Copy link
Contributor Author

mehmetf commented Jul 18, 2016

The screenshot was misleading. We are not trying to use a dropdown widget; we are simply trying to render something that looks like it with a custom onTap behavior:

                title: new InkWell(
                    onTap: () {},
                    child: new Container(
                            decoration: const BoxDecoration(
                                border: _kDropDownUnderline),
                            child: new Row(children: <Widget>[
                              new Text(gtString.companies),
                              new Icon(Icons.arrow_drop_down, size: 36.0)
                            ]))),

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

Ah, that makes sense. Try adding the following argument to your Row:

mainAxisSize: MainAxisSize.min

That will tell the row to try to be as small as possible in the horizontal direction (rather than being as big as possible, which is the default). If you combine that with an Align widget between your Container and you InkWell, that should give you the effect you're looking for.

@mehmetf
Copy link
Contributor Author

mehmetf commented Jul 18, 2016

Now that you mention it, we also had this issue with the dropdowns and we had to wrap them in Row() widgets as well.

image

I just wrapped the dropdowns with Align instead of Row and it worked. However, that trick still does not work for the Container example I gave.

In our experience with leafy's design, most of the time, we want the widget containers to wrap content:
E.g. dropdowns should size themselves according to the largest item in the list and not stretch to their parent.

In other words, it should not be necessary for you to wrap the dropdown button with Align; unless that's for actually aligning the child in the parent container.

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

Also, notice that the DropDownButton uses the mainAxisAlignment: MainAxisAlignment.spaceBetween attribute in its Row. That's what causes the arrow to stick to the right edge of the row as the row changes width (because the free space is put between the children). In your specific case, that doesn't matter because you want to collapse away all the free space anyway, but if you want to use your widget in more general situations, you might want to include that as well.

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

In other words, it should not be necessary for you to wrap the dropdown button with Align; unless that's for actually aligning the child in the parent container.

The screen doesn't shrinkwrap the widget (because the screen is a physical size), which means at some point the shrink wrapping widgets need to meet a container that doesn't shrink wrap. At that point, we need to decide where to place the child within the parent because they aren't the same size. Should we place the child on the left or on the right? Should we center the child?

The purpose of Align is to let you tell the widgets how the child ought to be placed. We probably should and an align parameter to Container to make it less verbose for you to specify what you want to happen.

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

I filed #4950 about adding convenience properties for alignment in Container.

@mehmetf
Copy link
Contributor Author

mehmetf commented Jul 18, 2016

Thanks Adam. That worked.

Your explanation makes sense but it is counter-intuitive for someone coming from Android background. If a widget expands, I would immediately think that it is because the widget is set to expand. Isn't that the case in most web component implementations as well? You are saying that sometimes widgets expand because parent doesn't know where to put it so it is stretched. I.e. default for align is: "justify".

IMO, this is not good because if I see a widget not correctly sized, I would not know whether the widget itself is pushing its boundaries (such as me failing to set Row mainAxisSize) or the parent itself is doing align: justify.

If, say, you were defaulting to left align and I wanted something to be centered, I could immediately see that and fix it. In that case, if the widget's size was incorrect, I would also immediately know that it is because the widget is trying to expand as much as possible.

@abarth
Copy link
Contributor

abarth commented Jul 18, 2016

If a widget expands, I would immediately think that it is because the widget is set to expand.

In this system, constraints flow down the tree and sizes flow up. That means a widget can be taking up a lot of space either because its parent told it to (which is what's happening here) or because it decided to be large.

Isn't that the case in most web component implementations as well?

Dunno. The situation is different here because HTMLElement all have lists of children whereas here we're talking about widgets that have a unique child.

You are saying that sometimes widgets expand because parent doesn't know where to put it so it is stretched.

In this case, the app bar widget is telling the title widget that it must be a certain width, just like the screen has told the app bar widget that it must be a certain width. That constrain keeps getting passed down the tree until a widget does something other than pass it through. For example, the Align widget loosens its incoming constraints for its child and the allocates the free space by positioning its child as requested.

I.e. default for align is: "justify".

There is not such thing as a default alignment. The default is for the parent to pass its constraints along to its child and then adopts its child's size, which necessarily matches its incoming constraints.

IMO, this is not good because if I see a widget not correctly sized, I would not know whether the widget itself is pushing its boundaries (such as me failing to set Row mainAxisSize) or the parent itself is doing align: justify.

Yeah, we should make that easier to debug.

If, say, you were defaulting to left align and I wanted something to be centered, I could immediately see that and fix it. In that case, if the widget's size was incorrect, I would also immediately know that it is because the widget is trying to expand as much as possible.

I agree that you could build a different system that worked in that way. However, there are other benefits to the current design that would be lost in that design. For example, the ability to force a child to be a specific size is very useful. If the child didn't need to respect its incoming constraints, we would lose that ability.

@mehmetf
Copy link
Contributor Author

mehmetf commented Jul 18, 2016

Thanks for the explanation Adam. Perhaps I will get used to this system. I highly recommend that you publish some sort of detailed explanation of this under Layout topic in the flutter website. I am sure many mobile developers will run into this.

I believe you can make this quite easy by providing a visual layout inspector similar to Pixel Perfect: https://developer.android.com/studio/profile/optimize-ui.html . This would allow seeing the actual boundaries of each widget. In addition to that, it would be nice for you to describe who is providing the constraints: child or parent.

I will close this bug for now and leave it up to you to create a feature request for such a tool.

@mehmetf mehmetf closed this as completed Jul 18, 2016
@Hixie
Copy link
Contributor

Hixie commented Jul 19, 2016

One trick you can use is to call debugDumpRenderTree() (I believe our Atom plugin has a way to do this for you) which will let you examine the constraints as they were passed down and the sizes and positions as they were sent back up. See also https://flutter.io/debugging/.

@Hixie
Copy link
Contributor

Hixie commented Jul 19, 2016

Follow-up bugs for improving docs: #3513 #3339
Follow-up bugs for debugging tools: #4967

@github-actions
Copy link

github-actions bot commented Sep 6, 2021

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 Sep 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

No branches or pull requests

4 participants