BoxLayout: Added set_collapse_when_hidden(), like views::Label.

ui/views/layout/box_layout.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/layout/box_layout.h"
 
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/view.h"
 
 namespace views {
 
 BoxLayout::BoxLayout(BoxLayout::Orientation orientation,
                      int inside_border_horizontal_spacing,
                      int inside_border_vertical_spacing,
                      int between_child_spacing)
     : orientation_(orientation),
       inside_border_insets_(inside_border_vertical_spacing,
                             inside_border_horizontal_spacing,
                             inside_border_vertical_spacing,
                             inside_border_horizontal_spacing),
       between_child_spacing_(between_child_spacing),
       main_axis_alignment_(MAIN_AXIS_ALIGNMENT_START),
       cross_axis_alignment_(CROSS_AXIS_ALIGNMENT_STRETCH),
       default_flex_(0),
       minimum_cross_axis_size_(0),
+      collapse_when_hidden_(false),
       host_(NULL) {
 }
 
 BoxLayout::~BoxLayout() {
 }
 
 void BoxLayout::SetFlexForView(const View* view, int flex_weight) {
   DCHECK(host_);
   DCHECK(view);
   DCHECK_EQ(host_, view->parent());
@@ skipping 113 matching lines @@
     child->SetBoundsRect(bounds);
   }
 
   // Flex views should have grown/shrunk to consume all free space.
   if (flex_sum)
     DCHECK_EQ(total_padding, main_free_space);
 }
 
 gfx::Size BoxLayout::GetPreferredSize(const View* host) const {
   DCHECK_EQ(host_, host);
+  if (!host->visible() && collapse_when_hidden_)
+    return gfx::Size();
+
   // Calculate the child views' preferred width.
   int width = 0;
   if (orientation_ == kVertical) {
     for (int i = 0; i < host->child_count(); ++i) {
       const View* child = host->child_at(i);
       if (!child->visible())
         continue;

[sadrul, 2015/10/29 17:57:29]

Most layout-managers currently explicitly check for child view's visibility
(like BoxLayout does here). Is there a good reason to change this?

[Matt Giuca, 2015/11/05 05:36:58]

I think you're right about this. But let's break it down.

Essentially, I'm doing this because GridLayout *does not* respect its children's
visibility (and instead requires the child to have GetPreferredSize return 0 or
it allocates space for them).

It looks like BoxLayout *does* respect its children's visibility, and that's the
"right" thing to do. (yes?)

Now while that's going on, Label has collapse_when_hidden(), seemingly to get
around the GridLayout issue. But you're suggesting that Label shouldn't have
this feature just to support GridLayout; instead GridLayout should respect its
child view's visibility?

Have I got everything right so far?

If that's the case, can we fix GridLayout? The steps would be:
1. Add an option to GridLayout collapse_children_when_hidden to assume children
have a size of 0x0 if they are invisible.
2. Update all code using Label::collapse_when_hidden to use
GridLayout::collapse_children_when_hidden instead.
3. Remove Label::collapse_when_hidden.
4. (potentially) Remove GridLayout::collapse_when_hidden and make this the
default behaviour.

Or we could just skip to Step 4 and test thoroughly --- it isn't likely to break
much.

In the meantime though, I might just update my particular view
(ExclusiveAccessBubbleView) to use BoxLayout instead so that I'm not blocked on
this.

[Matt Giuca, 2015/11/06 05:44:48]

Note: Just mailed out https://codereview.chromium.org/1410413007 which frees up
my dependency on GridLayout (so I no longer need this CL).

Keeping it open so we can finish this discussion, then I will close this since I
agree with you that propagating the collapse_when_hidden hack from Label is the
wrong thing to do.

 
       width = std::max(width, child->GetPreferredSize().width());
     }
     width = std::max(width, minimum_cross_axis_size_);
   }
 
   return GetPreferredSizeForChildWidth(host, width);
 }
 
 int BoxLayout::GetPreferredHeightForWidth(const View* host, int width) const {
@@ skipping 134 matching lines @@
                    child_area_bounds.height() + non_child_size.height());
 }
 
 gfx::Size BoxLayout::NonChildSize(const View* host) const {
   gfx::Insets insets(host->GetInsets());
   return gfx::Size(insets.width() + inside_border_insets_.width(),
                    insets.height() + inside_border_insets_.height());
 }
 
 } // namespace views