blob: 8613d124085fa27e7429fc63b2c131c26a2eda35 [file] [log] [blame] [view]
Wei Libc431812020-06-03 22:44:371# Best practice: layout
2
3The most important principles when working with Views layout are abstraction
4and encapsulation. The goal of the guidance here is to split the broad work of
5"layout" into many discrete pieces, each of which can be easily understood,
6tested, and changed in isolation. Compliant code should be more readable,
7performant, and maintainable; more adaptable to future modifications; and more
8stylistically consistent with other UI elements both now and in the future.
9
10[TOC]
11
12## Express layout values logically
13
14Both in mocks and code, **layout values should be described in functional terms
15that conform to the relevant specs** (particularly the
16[Material Refresh][] and older [Harmony][] specs). Rather than a mock saying
17a dialog title is "15 pt high", it should say it is the "Title 1" or
18"DIALOG\_TITLE" style; two non-interacting controls on the same line should not
19be separated by "16 dip" but by [`DISTANCE_UNRELATED_CONTROL_HORIZONTAL`][].
20Designers and engineers can reference a [dictionary][] to agree on common
21terminology. Don't simply back-figure the constant to use based on what
22produces the same value as the mock, as future design system changes will
23cause subtle and confusing bugs. Similarly, don't hardcode designer-provided
24values that aren't currently present in Chrome, as the semantics of such
25one-off values are unclear and will cause maintenance problems.
26Work with the Toolkit and UX teams to modify the design and overall spec so
27the desired results can be achieved in a centralized way.
28
29Note: the concept in this section is general, but the linked specs are
30**Googlers Only**.
31
32[Material Refresh]: https://docs.google.com/presentation/d/1EO7TOpIMJ7QHjaTVw9St-q6naKwtXX2TwzMirG5EsKY/edit#slide=id.g3232c09376_6_794
33[Harmony]: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20%28MD%29/Secondary%20UI%20Previews%20and%20specs%20%28exports%29/Spec
John Palmer046f9872021-05-24 01:24:5634[`DISTANCE_UNRELATED_CONTROL_HORIZONTAL`]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/chrome_layout_provider.h;l=56;drc=ec62c9ac3ef71a7014e27c5d2cf98917a89e3524
Wei Libc431812020-06-03 22:44:3735[dictionary]: http://go/DesktopDictionary
36
37## Obtain layout values from provider objects
38
39Once layout styles and values are expressed functionally, **the exact values
40should be obtained from relevant provider objects, not computed directly.**
41Code should not have any magic numbers for sizes, positions, elevations, and
42the like; this includes transformations like scaling values up or down by a
43fraction, or tweaking a provided value by a few DIPs. The most commonly used
44provider object is the [`LayoutProvider`][] (or its `chrome`/-side extension,
45[`ChromeLayoutProvider`][]); `View`s can obtain the global instance via a
46relevant [`Get()`][] method and ask it for relevant [distances][], [insets][],
47[corner radii][], [shadow elevations][], and the like. For text-setting
48controls like [`Label`][], the [`TypographyProvider`][] (or its
49`chrome`/-side extension, [`ChromeTypographyProvider`]),
50usually accessed via [global helper functions][], can provide appropriate
51[fonts][], [colors][], and [line heights][]. Most `View`s should not use these
52directly, but use `Label` and other such controls, providing the appropriate
53[context][] and [style][].
54
John Palmer046f9872021-05-24 01:24:5655[`LayoutProvider`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;l=129;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
56[`ChromeLayoutProvider`]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/chrome_layout_provider.h;drc=ec62c9ac3ef71a7014e27c5d2cf98917a89e3524;l=76
57[`Get()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38;l=135
58[distances]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38;l=148
59[insets]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38;l=144
60[corner radii]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38;l=168
61[shadow elevations]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_provider.h;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38;l=172
62[`Label`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/controls/label.h;l=30;drc=59135b4042aa469752899e8e4bf2a0a81d3d320c
63[`TypographyProvider`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography_provider.h;l=22;drc=b5e29e075e814ed41e6727c281b69f797d8a1e10
64[`ChromeTypographyProvider`]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/chrome_typography_provider.h;l=13;drc=a7ee000c95842e2dce6397ca36926924f4cb322b
65[global helper functions]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography.h;l=109;drc=8f7db479018a99e5906876954de93ae6d23bee58
66[fonts]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography_provider.h;l=28;drc=b5e29e075e814ed41e6727c281b69f797d8a1e10
67[colors]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography_provider.h;l=32;drc=b5e29e075e814ed41e6727c281b69f797d8a1e10
68[line heights]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography_provider.h;l=37;drc=b5e29e075e814ed41e6727c281b69f797d8a1e10
69[context]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography.h;l=23;drc=8f7db479018a99e5906876954de93ae6d23bee58
70[style]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/style/typography.h;l=67;drc=8f7db479018a99e5906876954de93ae6d23bee58
Wei Libc431812020-06-03 22:44:3771
72|||---|||
73
74#####
75
76**Avoid**
77
78[Current code][1] uses file scoped hard-coded padding values for its layout
79constants.
80
John Palmer046f9872021-05-24 01:24:5681[1]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/subtle_notification_view.cc;l=142;drc=787d0aacc071674dc83f6059072d15f8cfffbf84
Wei Libc431812020-06-03 22:44:3782
83#####
84
85**Best practice**
86
87A better approach would be to use layout constants sourced from the
88[`ChromeLayoutProvider`][].
89
90|||---|||
91
92|||---|||
93
94#####
95
96``` cpp
97namespace {
98// Space between the site info label.
99const int kMiddlePaddingPx = 30;
100
101const int kOuterPaddingHorizPx = 40;
102const int kOuterPaddingVertPx = 8;
103} // namespace
104
105SubtleNotificationView::SubtleNotificationView()
106 : instruction_view_(nullptr) {
107 ...
108 instruction_view_ =
Jan Wilken Dörrie85285b02021-03-11 23:38:47109 new InstructionView(std::u16string());
Wei Libc431812020-06-03 22:44:37110
111 int outer_padding_horiz = kOuterPaddingHorizPx;
112 int outer_padding_vert = kOuterPaddingVertPx;
113 AddChildView(instruction_view_);
114
115 SetLayoutManager(std::make_unique<views::BoxLayout>(
116 views::BoxLayout::Orientation::kHorizontal,
117 gfx::Insets(outer_padding_vert,
118 outer_padding_horiz),
119 kMiddlePaddingPx));
120}
121```
122
123#####
124
125``` cpp
126
127
128
129
130
131
132
133
134SubtleNotificationView::SubtleNotificationView()
135 : instruction_view_(nullptr) {
136 ...
137 AddChildView(std::make_unique<InstructionView>(
Jan Wilken Dörrie85285b02021-03-11 23:38:47138 std::u16string()));
Wei Libc431812020-06-03 22:44:37139
140 const gfx::Insets kDialogInsets =
141 ChromeLayoutProvider::Get()->GetInsetsMetric(
142 views::INSETS_DIALOG);
143 const int kHorizontalPadding =
144 ChromeLayoutProvider::Get()->GetDistanceMetric(
145 views::DISTANCE_RELATED_LABEL_HORIZONTAL);
146 SetLayoutManager(std::make_unique<views::BoxLayout>(
147 views::BoxLayout::Orientation::kHorizontal,
148 kDialogInsets, kHorizontalPadding));
149}
150```
151
152|||---|||
153
154
155## Use hierarchy liberally
156
157While not a layout-specific tactic, it simplifies many layout issues
158**to break a high-level UI construct into a hierarchy of `View`s**,
159with as many levels as necessary to make each `View` as simple as possible.
160In such hierarchies, most non-leaf `View`s will be nameless "containers" that
161simply size or group their immediate children, perhaps with padding between
162them or a margin around the outside. Each such `View` is easy to lay out,
163and you can later combine or factor out pieces of the hierarchy as appropriate,
164including adding helpers for common Material Design idioms to the core toolkit.
165
166
167## Use LayoutManagers
168
169**Avoid overriding [`Layout()`][] to programmatically lay out children.**
170In nearly all cases, the built-in [`LayoutManager`][]s can achieve the desired
171layout, and do so in a declarative rather than imperative fashion. The
172resulting code is often simpler and easier to understand. Writing a [bespoke
173`LayoutManager`][] is also possible, but less common.
174
John Palmer046f9872021-05-24 01:24:56175[`Layout()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.h;l=730;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
176[`LayoutManager`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/layout_manager.h;l=33;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
177[bespoke `LayoutManager`]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/try_chrome_dialog_win/button_layout.h;l=30;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
Wei Libc431812020-06-03 22:44:37178
179|||---|||
180
181#####
182
183**Avoid**
184
185The following old code used Layout() to have its label text fill the dialog.
186
187#####
188
189**Best practice**
190
191[Current code][2] uses a [FillLayout][] to achieve the same result.
192
John Palmer046f9872021-05-24 01:24:56193[2]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/relaunch_notification/relaunch_required_dialog_view.cc;l=91;drc=1ec33e7c19e2d63b3f918df115c12f77f419645b
194[FillLayout]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/fill_layout.h
Wei Libc431812020-06-03 22:44:37195
196|||---|||
197
198|||---|||
199
200#####
201
202``` cpp
203int RelaunchRequiredDialogView::GetHeightForWidth(
204 int width) const {
205 const gfx::Insets insets = GetInsets();
206 return body_label_->GetHeightForWidth(
207 width - insets.width()) + insets.height();
208}
209
Peter Kasting1e3a1e992024-02-01 18:10:41210void RelaunchRequiredDialogView::Layout(PassKey) {
Wei Libc431812020-06-03 22:44:37211 body_label_->SetBoundsRect(GetContentsBounds());
212}
213```
214
215#####
216
217``` cpp
218RelaunchRequiredDialogView::RelaunchRequiredDialogView(
219 base::Time deadline,
220 base::RepeatingClosure on_accept)
221 : ...{
222 SetLayoutManager(
223 std::make_unique<views::FillLayout>());
224 ...
225}
226
227
228```
229
230|||---|||
231
232
233## Prefer intrinsic constraints to extrinsic computation
234
235Where possible, **express the desired outcome of layout in terms of intrinsic
236constraints for each `View`,** instead of trying to conditionally compute
237the desired output metrics. For example, using a [`ClassProperty`][]
238to set each child's [margins][] is less error-prone than trying to
239conditionally add padding `View`s between children. When coupled with
240[margin collapsing][] and [internal padding][], it's possible to do things
241like use [different padding amounts between different children][]
242or visually align elements without manually computing offsets.
243Such manual computation is prone to bugs if someone changes a size, padding
244value, or child order in one place without also updating related computations
245elsewhere.
246
John Palmer046f9872021-05-24 01:24:56247[`ClassProperty`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/base/class_property.h;l=55;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
248[margins]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view_class_properties.h;l=30;drc=1449b8c60358c4cdea1722e4c1e8079bd1b5f306
249[margin collapsing]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/flex_layout.h;l=87;drc=62bf27aca5418212ceadd8daf9188d2aa437bfcc
250[internal padding]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view_class_properties.h;l=40;drc=1449b8c60358c4cdea1722e4c1e8079bd1b5f306
251[different padding amounts between different children]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/toolbar/toolbar_view.cc;l=974;drc=34a8c4215229379ced3586125399c7ad3c65b87f
Wei Libc431812020-06-03 22:44:37252
253|||---|||
254
255#####
256
257**Avoid**
258
259The following is old code that calculated bubble padding through calculations
260involving the control insets.
261
262#####
263
264**Best practice**
265
266[Current code][3] uses a combination of margin and padding on the
267ColorPickerView to ensure proper alignment.
268
John Palmer046f9872021-05-24 01:24:56269[3]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.cc;l=89;drc=542c4c6ac89bc665807351d3fb4aca5ebddc82f8
Wei Libc431812020-06-03 22:44:37270
271|||---|||
272
273|||---|||
274
275#####
276
277``` cpp
278TabGroupEditorBubbleView::TabGroupEditorBubbleView(
279 const Browser* browser,
280 const tab_groups::TabGroupId& group,
281 TabGroupHeader* anchor_view,
David Benjamin1d7374822024-07-09 19:54:35282 std::optional<gfx::Rect> anchor_rect,
Wei Libc431812020-06-03 22:44:37283 bool stop_context_menu_propagation)
284 : ... {
285
286 ...
287 const auto* layout_provider =
288 ChromeLayoutProvider::Get();
289 const int horizontal_spacing =
290 layout_provider->GetDistanceMetric(
291 views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
292 const int vertical_menu_spacing =
293 layout_provider->GetDistanceMetric(
294 views::DISTANCE_RELATED_CONTROL_VERTICAL);
295
296 // The vertical spacing for the non menu items within
297 // the editor bubble.
298 const int vertical_dialog_content_spacing = 16;
299
300
301
302
303
304
305
306
307
308
309 views::View* group_modifier_container =
310 AddChildView(std::make_unique<views::View>());
311
312 gfx::Insets color_element_insets =
313 ChromeLayoutProvider::Get()->GetInsetsMetric(
314 views::INSETS_VECTOR_IMAGE_BUTTON);
315 group_modifier_container->SetBorder(
316 views::CreateEmptyBorder(
317 gfx::Insets(vertical_dialog_content_spacing,
318 horizontal_spacing -
319 color_element_insets.left(),
320 vertical_dialog_content_spacing,
321 horizontal_spacing -
322 color_element_insets.right())));
323 ...
324 // Add the text field for editing the title.
325 views::View* title_field_container =
326 group_modifier_container->AddChildView(
327 std::make_unique<views::View>());
328 title_field_container->SetBorder(
329 views::CreateEmptyBorder(gfx::Insets(
330 0, color_element_insets.left(),
331 vertical_dialog_content_spacing,
332 color_element_insets.right()))
333 ...
334 color_selector_ =
335 group_modifier_container->AddChildView(
336 std::make_unique<ColorPickerView>(
337 colors_, background_color(), initial_color,
Ayu Ishiib54dedb2021-02-03 16:19:55338 base::BindRepeating(
Wei Libc431812020-06-03 22:44:37339 &TabGroupEditorBubbleView::UpdateGroup,
340 base::Unretained(this))));
341 ...
342}
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368```
369
370#####
371
372``` cpp
373TabGroupEditorBubbleView::TabGroupEditorBubbleView(
374 const Browser* browser,
375 const tab_groups::TabGroupId& group,
376 views::View* anchor_view,
David Benjamin1d7374822024-07-09 19:54:35377 std::optional<gfx::Rect> anchor_rect,
Wei Libc431812020-06-03 22:44:37378 TabGroupHeader* header_view,
379 bool stop_context_menu_propagation)
380 : ... {
381 ...
382 const auto* layout_provider =
383 ChromeLayoutProvider::Get();
384 const int horizontal_spacing =
385 layout_provider->GetDistanceMetric(
386 views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
387 const int vertical_spacing =
388 layout_provider->GetDistanceMetric(
389 views::DISTANCE_RELATED_CONTROL_VERTICAL);
390
391 // The padding of the editing controls is adaptive,
392 // to improve the hit target size and screen real
393 // estate usage on touch devices.
394 const int group_modifier_vertical_spacing =
395 ui::TouchUiController::Get()->touch_ui() ?
396 vertical_spacing / 2 : vertical_spacing;
397 const gfx::Insets control_insets =
398 ui::TouchUiController::Get()->touch_ui()
399 ? gfx::Insets(5 * vertical_spacing / 4,
400 horizontal_spacing)
401 : gfx::Insets(vertical_spacing,
402 horizontal_spacing);
403
404 views::View* group_modifier_container =
405 AddChildView(std::make_unique<views::View>());
406 group_modifier_container->SetBorder(
407 views::CreateEmptyBorder(gfx::Insets(
408 group_modifier_vertical_spacing, 0)));
409
410 views::FlexLayout* group_modifier_container_layout =
411 group_modifier_container->SetLayoutManager(
412 std::make_unique<views::FlexLayout>());
413 group_modifier_container_layout
414 ->SetOrientation(
415 views::LayoutOrientation::kVertical)
416 .SetIgnoreDefaultMainAxisMargins(true);
417
418
419 // Add the text field for editing the title.
420 views::View* title_field_container =
421 group_modifier_container->AddChildView(
422 std::make_unique<views::View>());
423 title_field_container->SetBorder(
424 views::CreateEmptyBorder(
425 control_insets.top(), control_insets.left(),
426 group_modifier_vertical_spacing,
427 control_insets.right()));
428
429 ...
430 const tab_groups::TabGroupColorId initial_color_id =
431 InitColorSet();
432 color_selector_ =
433 group_modifier_container->AddChildView(
434 std::make_unique<ColorPickerView>(
435 this, colors_, initial_color_id,
Ayu Ishiib54dedb2021-02-03 16:19:55436 base::BindRepeating(
Wei Libc431812020-06-03 22:44:37437 &TabGroupEditorBubbleView::UpdateGroup,
438 base::Unretained(this))));
439 color_selector_->SetProperty(
440 views::kMarginsKey,
441 gfx::Insets(0, control_insets.left(), 0,
442 control_insets.right()));
443 ...
444}
445
446ColorPickerView::ColorPickerView(
447 const views::BubbleDialogDelegateView* bubble_view,
448 const TabGroupEditorBubbleView::Colors& colors,
449 tab_groups::TabGroupColorId initial_color_id,
450 ColorSelectedCallback callback)
451 : callback_(std::move(callback)) {
452 ...
453 // Set the internal padding to be equal to the
454 // horizontal insets of a color picker element,
455 // since that is the amount by which the color
456 // picker's margins should be adjusted to make it
457 // visually align with other controls.
458 gfx::Insets child_insets = elements_[0]->GetInsets();
459 SetProperty(views::kInternalPaddingKey,
460 gfx::Insets(0, child_insets.left(), 0,
461 child_insets.right()));
462}
463```
464
465|||---|||
466
Peter Kasting2ead0ee2021-10-20 16:55:27467## Use TableLayout with caution
Wei Libc431812020-06-03 22:44:37468
Peter Kasting2ead0ee2021-10-20 16:55:27469[`TableLayout`][] is a `LayoutManager` used for tabular layouts. Much like
Wei Libc431812020-06-03 22:44:37470table-based layout in HTML, it can achieve almost any desired effect, and in
471some scenarios (e.g. creating an actual table) is the best tool. Used
472indiscriminately, it can be cryptic, verbose, and error-prone. Accordingly,
Peter Kasting2ead0ee2021-10-20 16:55:27473**use `TableLayout` only when creating a true grid or table, not simply for
474selective horizontal and vertical alignment.** For simple layouts,
475[`BoxLayout`][] and [`FlexLayout`][] are better choices; for more complex
476layouts, representing sections or groups hierarchically may result in simpler
477inner layouts that can be nested within an overall layout.
Wei Libc431812020-06-03 22:44:37478
Peter Kasting2ead0ee2021-10-20 16:55:27479[`TableLayout`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/table_layout.h;l=73;drc=f513afe81fca508d22153b192f1fab33e2c444fa
John Palmer046f9872021-05-24 01:24:56480[`BoxLayout`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/box_layout.h;l=28;drc=5b9e43d976aca377588875fc59c5348ede02a8b5
481[`FlexLayout`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/layout/flex_layout.h;l=73;drc=62bf27aca5418212ceadd8daf9188d2aa437bfcc
Wei Libc431812020-06-03 22:44:37482
483|||---|||
484
485#####
486
487**Avoid**
488
Peter Kasting2ead0ee2021-10-20 16:55:27489The following old code uses a [`TableLayout`][] to create a HoverButton with
Wei Libc431812020-06-03 22:44:37490a stacked title and subtitle flanked on by views on both sides.
491
492#####
493
494**Best practice**
495
496[Current code][4] uses [`FlexLayout`][] to achieve the desired result, resulting
497in clearer code.
498
Peter Kastingf4b162e2024-02-08 16:25:48499[4]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/controls/hover_button.cc;l=129;drc=0139ceffb2f8e1f64b7c30834e57d5793e529ed7
Wei Libc431812020-06-03 22:44:37500
501|||---|||
502
503|||---|||
504
505#####
506
507``` cpp
508// Used to create the following layout
509// +-----------+---------------------+----------------+
510// | icon_view | title | secondary_view |
511// +-----------+---------------------+----------------+
512// | | subtitle | |
513// +-----------+---------------------+----------------+
514HoverButton::HoverButton(
Peter Kasting7e3f3542020-11-04 20:37:29515 ...
Wei Libc431812020-06-03 22:44:37516 std::unique_ptr<views::View> icon_view,
Jan Wilken Dörrie85285b02021-03-11 23:38:47517 const std::u16string& title,
518 const std::u16string& subtitle,
Wei Libc431812020-06-03 22:44:37519 std::unique_ptr<views::View> secondary_view,
Peter Kasting7e3f3542020-11-04 20:37:29520 ...) {
Wei Libc431812020-06-03 22:44:37521 ...
Peter Kasting2ead0ee2021-10-20 16:55:27522 views::TableLayout* table_layout =
Wei Libc431812020-06-03 22:44:37523 SetLayoutManager(
Peter Kasting2ead0ee2021-10-20 16:55:27524 std::make_unique<views::TableLayout>());
Wei Libc431812020-06-03 22:44:37525 ...
Peter Kasting2ead0ee2021-10-20 16:55:27526 table_layout->AddColumn(
527 views::LayoutAlignment::kCenter,
528 views::LayoutAlignment::kCenter,
529 views::TableLayout::kFixedSize,
530 views::TableLayout::kUsePreferred, 0, 0);
531 table_layout->AddPaddingColumn(
532 views::TableLayout::kFixedSize,
Wei Libc431812020-06-03 22:44:37533 icon_label_spacing);
Peter Kasting2ead0ee2021-10-20 16:55:27534 table_layout->AddColumn(
535 views::LayoutAlignment::kStretch,
536 views::LayoutAlignment::kStretch, 1.0f,
537 views::TableLayout::kUsePreferred, 0, 0);
Wei Libc431812020-06-03 22:44:37538 ...
Peter Kasting2ead0ee2021-10-20 16:55:27539 table_layout->AddRows(
540 1, views::TableLayout::kFixedSize,
Wei Libc431812020-06-03 22:44:37541 row_height);
Peter Kasting2ead0ee2021-10-20 16:55:27542 icon_view_ = AddChildView(
Wei Libc431812020-06-03 22:44:37543 std::move(icon_view), 1, num_labels);
544 ...
545 auto title_wrapper =
546 std::make_unique<SingleLineStyledLabelWrapper>(
547 title);
548 title_ = title_wrapper->label();
Peter Kasting2ead0ee2021-10-20 16:55:27549 AddChildView(std::move(title_wrapper));
Wei Libc431812020-06-03 22:44:37550
551 if (secondary_view) {
Peter Kasting2ead0ee2021-10-20 16:55:27552 table_layout->AddColumn(
553 views::LayoutAlignment::kCenter,
554 views::LayoutAlignment::kCenter,
555 views::TableLayout::kFixedSize,
556 views::TableLayout::kUsePreferred, 0, 0);
Wei Libc431812020-06-03 22:44:37557 ...
Peter Kasting2ead0ee2021-10-20 16:55:27558 secondary_view_ = AddChildView(
559 std::move(secondary_view), 1, num_labels);
Wei Libc431812020-06-03 22:44:37560 ...
561 }
562 if (!subtitle.empty()) {
Peter Kasting2ead0ee2021-10-20 16:55:27563 table_layout->AddRows(
564 1, views::TableLayout::kFixedSize,
Wei Libc431812020-06-03 22:44:37565 row_height);
566 auto subtitle_label =
567 std::make_unique<views::Label>(
568 subtitle, views::style::CONTEXT_BUTTON,
569 views::style::STYLE_SECONDARY);
570 ...
Peter Kasting2ead0ee2021-10-20 16:55:27571 AddChildView(std::make_unique<views::View>());
Wei Libc431812020-06-03 22:44:37572 subtitle_ =
Peter Kasting2ead0ee2021-10-20 16:55:27573 AddChildView(std::move(subtitle_label));
Wei Libc431812020-06-03 22:44:37574 }
575 ...
576}
Peter Kasting2ead0ee2021-10-20 16:55:27577
578
579
580
Wei Libc431812020-06-03 22:44:37581```
582
583#####
584
585``` cpp
586// Used to create the following layout
587// +-----------+---------------------+----------------+
588// | | title | |
589// | icon_view |---------------------| secondary_view |
590// | | subtitle | |
591// +-----------+---------------------+----------------+
592HoverButton::HoverButton(
Peter Kasting7e3f3542020-11-04 20:37:29593 ...
Wei Libc431812020-06-03 22:44:37594 std::unique_ptr<views::View> icon_view,
Jan Wilken Dörrie85285b02021-03-11 23:38:47595 const std::u16string& title,
596 const std::u16string& subtitle,
Wei Libc431812020-06-03 22:44:37597 std::unique_ptr<views::View> secondary_view,
Peter Kasting7e3f3542020-11-04 20:37:29598 ...) {
Wei Libc431812020-06-03 22:44:37599 ...
Peter Kastingf4b162e2024-02-08 16:25:48600 // Set the layout manager to ignore the
601 // ink_drop_container to ensure the ink drop tracks
602 // the bounds of its parent.
603 ink_drop_container()->SetProperty(
604 views::kViewIgnoredByLayoutKey, true);
605
Wei Libc431812020-06-03 22:44:37606 SetLayoutManager(
607 std::make_unique<views::FlexLayout>())
608 ->SetCrossAxisAlignment(
Peter Kastingf4b162e2024-02-08 16:25:48609 views::LayoutAlignment::kCenter);
Wei Libc431812020-06-03 22:44:37610 ...
611 icon_view_ =
612 AddChildView(std::make_unique<IconWrapper>(
613 std::move(icon_view), vertical_spacing))
614 ->icon();
615
616 // |label_wrapper| will hold both the title and
617 // subtitle if it exists.
618 auto label_wrapper = std::make_unique<views::View>();
619 title_ = label_wrapper->AddChildView(
620 std::make_unique<views::StyledLabel>(
621 title, nullptr));
622
623 if (!subtitle.empty()) {
624 auto subtitle_label =
625 std::make_unique<views::Label>(
626 subtitle, views::style::CONTEXT_BUTTON,
627 views::style::STYLE_SECONDARY);
628 ...
629 subtitle_ = label_wrapper->AddChildView(
630 std::move(subtitle_label));
631 }
632
633 label_wrapper->SetLayoutManager(
634 std::make_unique<views::FlexLayout>())
635 ->SetOrientation(
636 views::LayoutOrientation::kVertical)
637 .SetMainAxisAlignment(
638 views::LayoutAlignment::kCenter);
639 label_wrapper->SetProperty(
640 views::kFlexBehaviorKey,
641 views::FlexSpecification(
642 views::MinimumFlexSizeRule::kScaleToZero,
643 views::MaximumFlexSizeRule::kUnbounded));
644 label_wrapper->SetProperty(
645 views::kMarginsKey,
646 gfx::Insets(vertical_spacing, 0));
647 label_wrapper_ =
648 AddChildView(std::move(label_wrapper));
649 ...
650
651 if (secondary_view) {
652 ...
653 secondary_view->SetProperty(
654 views::kMarginsKey,
655 gfx::Insets(secondary_spacing,
656 icon_label_spacing,
657 secondary_spacing, 0));
658 secondary_view_ =
659 AddChildView(std::move(secondary_view));
660 }
661 ...
662}
663```
664
665|||---|||
666
667## Compute preferred/minimum sizes recursively from children
668
669**Avoid hardcoding preferred or minimum sizes,** including via metrics like
670[`DISTANCE_BUBBLE_PREFERRED_WIDTH`][].
671In many cases, `LayoutManager`s will provide reasonable values for these,
672and common codepaths like [`BubbleFrameView::GetFrameWidthForClientWidth()`][]
673can help ensure that the returned values are [conformed to spec][].
674When a `View` does need to calculate these manually, it should do so based on
675the corresponding values returned by its children, not by returning specific
676numbers (e.g. dialog preferred size is 300 by 150). In particular, assuming
677fonts will be in a certain size, or that a given fixed area is sufficient to
678display all necessary information, can cause hard-to-find localization and
679accessibility bugs for users with verbose languages or unusually large fonts.
680
John Palmer046f9872021-05-24 01:24:56681[`DISTANCE_BUBBLE_PREFERRED_WIDTH`]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/chrome_layout_provider.h;l=68;drc=ec62c9ac3ef71a7014e27c5d2cf98917a89e3524
682[`BubbleFrameView::GetFrameWidthForClientWidth()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/bubble/bubble_frame_view.cc;l=688;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
683[conformed to spec]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/bubble/bubble_frame_view.cc;l=698;drc=f5df5da5753795298349b2dd6325e2c5e6e13e38
Wei Libc431812020-06-03 22:44:37684
685|||---|||
686
687#####
688
689**Avoid**
690
691Current code overloads CalculatePreferredSize() in the dialog view.
692
693#####
694
695**Best practice**
696
697A better approach would be to omit the overload completely and let leaf views
698size the dialog appropriately, relying on the minimum size fallbacks
699if necessary.
700
701|||---|||
702
703|||---|||
704
705#####
706
707``` cpp
708...
709gfx::Size
710CastDialogView::CalculatePreferredSize() const {
711 const int width =
712 ChromeLayoutProvider::Get()->GetDistanceMetric(
713 DISTANCE_BUBBLE_PREFERRED_WIDTH);
714 return gfx::Size(width, GetHeightForWidth(width));
715}
716```
717
718#####
719
720``` cpp
721...
Peter Kasting2ead0ee2021-10-20 16:55:27722
723
724
725
726
727
728
Wei Libc431812020-06-03 22:44:37729```
730
731|||---|||
732
733
734## Handle events directly, not via Layout()
735
736In addition to using `LayoutManager`s in place of manual layout,
737**avoid overriding `Layout()` to perform non-layout actions.** For example,
738instead of updating properties tied to a `View`'s size in `Layout()`,
739do so in [`OnBoundsChanged()`][];
740when the `View` in question is a child, make the child a `View` subclass
741with an `OnBoundsChanged()` override instead of having the parent both lay
742the child out and update its properties. Modify the
743[hit-testing and event-handling functions][] directly instead of laying out
744invisible `View`s to intercept events. Toggle child visibility directly in
745response to external events rather than calculating it inside `Layout()`.
746
John Palmer046f9872021-05-24 01:24:56747[`OnBoundsChanged()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.h;l=1377;drc=34a8c4215229379ced3586125399c7ad3c65b87f
748[hit-testing and event-handling functions]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.h;l=919;drc=34a8c4215229379ced3586125399c7ad3c65b87f
Wei Libc431812020-06-03 22:44:37749
750|||---|||
751
752#####
753
754**Avoid**
755
756Old code updated hit testing and button properties in the Layout() method.
757
758#####
759
760**Best practice**
761
762Current code wraps the buttons in a file scoped class with an
763OnBoundsChanged() method and modifies the hit testing functions directly to
764achieve the same result.
765
766|||---|||
767
768|||---|||
769
770#####
771
772``` cpp
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816FindBarView::FindBarView(FindBarHost* host)
817 : find_bar_host_(host) {
818 auto find_text = std::make_unique<views::Textfield>();
819 find_text_ = AddChildView(std::move(find_text));
820 ...
821 auto find_previous_button =
822 views::CreateVectorImageButton(this);
823 find_previous_button_ =
824 AddChildView(std::move(find_previous_button));
825 ...
826 auto find_next_button =
827 views::CreateVectorImageButton(this);
828 find_next_button_ =
829 AddChildView(std::move(find_next_button));
830 ...
831 auto close_button =
832 views::CreateVectorImageButton(this);
833 close_button_ =
834 AddChildView(std::move(close_button));
835}
836
Peter Kasting1e3a1e992024-02-01 18:10:41837void FindBarView::Layout(PassKey) {
Peter Kasting8e6535e2024-01-29 23:05:39838 LayoutSuperclass<views::View>(this);
Wei Libc431812020-06-03 22:44:37839 // The focus forwarder view is a hidden view that
840 // should cover the area between the find text box
841 // and the find button so that when the user clicks
842 // in that area we focus on the find text box.
843 const int find_text_edge =
844 find_text_->x() + find_text_->width();
845 focus_forwarder_view_->SetBounds(
846 find_text_edge, find_previous_button_->y(),
847 find_previous_button_->x() - find_text_edge,
848 find_previous_button_->height());
849
850 for (auto* button :
851 {find_previous_button_, find_next_button_,
852 close_button_}) {
853 constexpr int kCircleDiameterDp = 24;
854 auto highlight_path = std::make_unique<SkPath>();
855 // Use a centered circular shape for inkdrops and
856 // focus rings.
857 gfx::Rect circle_rect(button->GetLocalBounds());
858 circle_rect.ClampToCenteredSize(
859 gfx::Size(kCircleDiameterDp,
860 kCircleDiameterDp));
861 highlight_path->addOval(
862 gfx::RectToSkRect(circle_rect));
863 button->SetProperty(views::kHighlightPathKey,
864 highlight_path.release());
865 }
866}
867```
868
869#####
870
871``` cpp
872// An ImageButton that has a centered circular
873// highlight.
874class FindBarView::FindBarButton
875 : public views::ImageButton {
876 public:
877 using ImageButton::ImageButton;
878 protected:
879 void OnBoundsChanged(
880 const gfx::Rect& previous_bounds) override {
881 const gfx::Rect bounds = GetLocalBounds();
882 auto highlight_path = std::make_unique<SkPath>();
883 const gfx::Point center = bounds.CenterPoint();
884 const int radius = views::LayoutProvider::Get()
885 ->GetCornerRadiusMetric(
Hwanseung Leeda3a7a82021-04-08 23:34:07886 views::Emphasis::kMaximum, bounds.size());
Wei Libc431812020-06-03 22:44:37887 highlight_path->addCircle(
888 center.x(), center.y(), radius);
889 SetProperty(views::kHighlightPathKey,
890 highlight_path.release());
891 }
892};
893
894bool FindBarView::OnMousePressed(
895 const ui::MouseEvent& event) {
896 // The find text box only extends to the match count
897 // label. However, users expect to be able to click
898 // anywhere inside what looks like the find text
899 // box (including on or around the match_count label)
900 // and have focus brought to the find box. Cause
901 // clicks between the textfield and the find previous
902 // button to focus the textfield.
903 const int find_text_edge =
904 find_text_->bounds().right();
905 const gfx::Rect focus_area(
906 find_text_edge, find_previous_button_->y(),
907 find_previous_button_->x() - find_text_edge,
908 find_previous_button_->height());
909 if (!GetMirroredRect(focus_area).Contains(
910 event.location()))
911 return false;
912 find_text_->RequestFocus();
913 return true;
914
915FindBarView::FindBarView(FindBarHost* host)
916 : find_bar_host_(host) {
917 auto find_text = std::make_unique<views::Textfield>();
918 find_text_ = AddChildView(std::move(find_text));
919 ...
920 auto find_previous_button =
921 std::make_unique<FindBarButton>(this);
922 views::ConfigureVectorImageButton(
923 find_previous_button.get());
924 ...
925 auto find_next_button =
926 std::make_unique<FindBarButton>(this);
927 views::ConfigureVectorImageButton(
928 find_next_button.get());
929 ...
930 auto close_button =
931 std::make_unique<FindBarButton>(this);
932 views::ConfigureVectorImageButton(close_button.get());
933}
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966```
967
968|||---|||
969
Peter Kastingdf36f33e2024-01-31 19:21:34970## Don't invoke DeprecatedLayoutImmediately()
Wei Libc431812020-06-03 22:44:37971
Peter Kastingdf36f33e2024-01-31 19:21:34972**Avoid calls to `DeprecatedLayoutImmediately()`.**
Wei Libc431812020-06-03 22:44:37973These are typically used for three purposes:
974
Peter Kastingdf36f33e2024-01-31 19:21:349751. *Calling `DeprecatedLayoutImmediately()` on `this`, when something that
976affects layout has changed.* This forces a synchronous layout, which can lead to
977needless work (e.g. if several sequential changes each trigger layout). Use
Wei Libc431812020-06-03 22:44:37978asynchronous layout\* instead. In many cases (such as
979[the preferred size changing][] or
980[a child needing layout][],
981a `View` will automatically mark itself as needing layout; when necessary, call
982[`InvalidateLayout()`][] to mark it manually.
983
Peter Kastingdf36f33e2024-01-31 19:21:349841. *Calling `DeprecatedLayoutImmediately()` or `InvalidateLayout()` on some
985`View` to notify it that something affecting its layout has changed.* Instead,
986ensure that `View` is notified of the underlying change (via specific method
987overrides or plumbing from a model object), and then invalidates its own layout
988when needed.
Wei Libc431812020-06-03 22:44:37989
Peter Kastingdf36f33e2024-01-31 19:21:349901. *Calling `DeprecatedLayoutImmediately()` on some `View` to "ensure it's up
991to date" before reading some layout-related property off it.* Instead, plumb any
992relevant events to the current object, then handle them directly (e.g. override
Wei Libc431812020-06-03 22:44:37993[`ChildPreferredSizeChanged()`][] or use a [`ViewObserver`][]
994to monitor the target `View`; then update local state as necessary and trigger
995handler methods).
996
997\* *How does asynchronous layout work?* In the browser, the compositor
998periodically [requests a LayerTreeHost update][].
999This ultimately calls back to [`Widget::LayoutRootViewIfNecessary()`][],
1000recursively laying out invalidated `View`s within the `Widget`. In unittests,
1001this compositor-driven sequence never occurs, so it's necessary to
Peter Kasting94c0f2e2024-01-29 18:14:201002[call RunScheduledLayout() manually][] when a test needs to ensure a `View`'s
1003layout is up-to-date. Many tests fail to do this, but currently pass because
1004something triggers Layout() directly; accordingly, changing existing code from
1005synchronous to asynchronous layout may require adding `RunScheduledLayout()`
1006calls to (possibly many) tests, and this is not a sign that the change is wrong.
Wei Libc431812020-06-03 22:44:371007
John Palmer046f9872021-05-24 01:24:561008[the preferred size changing]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.cc;l=1673;drc=bc9a6d40468646be476c61b6637b51729bec7b6d
1009[a child needing layout]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.cc;l=777;drc=bc9a6d40468646be476c61b6637b51729bec7b6d
1010[`InvalidateLayout()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.h;l=735;drc=c06f6b339b47ce2388624aa9a89334ace38a71e4
1011[`ChildPreferredSizeChanged()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view.h;l=1381;drc=c06f6b339b47ce2388624aa9a89334ace38a71e4
1012[`ViewObserver`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/view_observer.h;l=17;drc=eb20fd77330dc4a89eecf17459263e5895e7f177
1013[requests a LayerTreeHost update]: https://source.chromium.org/chromium/chromium/src/+/main:cc/trees/layer_tree_host.cc;l=304;drc=c06f6b339b47ce2388624aa9a89334ace38a71e4
1014[`Widget::LayoutRootViewIfNecessary()`]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/widget/widget.h;l=946;drc=b1dcb398c454a576092d38d0d67db3709b2b2a9b
Peter Kasting94c0f2e2024-01-29 18:14:201015[call RunScheduledLayout() manually]: https://source.chromium.org/chromium/chromium/src/+/main:ui/views/test/views_test_utils.h;l=17;drc=3e1a26c44c024d97dc9a4c09bbc6a2365398ca2c
Wei Libc431812020-06-03 22:44:371016
1017|||---|||
1018
1019#####
1020
1021**Avoid**
1022
Peter Kastingdf36f33e2024-01-31 19:21:341023[Current code][5] makes an unnecessary call to DeprecatedLayoutImmediately()
Wei Libc431812020-06-03 22:44:371024
Peter Kastingdf36f33e2024-01-31 19:21:341025[5]: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/media_router/cast_dialog_view.cc;l=283;drc=7db01ff4534c04419f3fa10d75e0b97b0a5a4f99
Wei Libc431812020-06-03 22:44:371026
1027#####
1028
1029**Best practice**
1030
1031A better approach would be to call InvalidateLayout() and update the necessary tests.
1032
1033|||---|||
1034
1035|||---|||
1036
1037#####
1038
1039``` cpp
1040void CastDialogView::PopulateScrollView(
1041 const std::vector<UIMediaSink>& sinks) {
1042 ...
Peter Kastingdf36f33e2024-01-31 19:21:341043 DeprecatedLayoutImmediately();
Wei Libc431812020-06-03 22:44:371044}
1045
1046TEST_F(CastDialogViewTest, PopulateDialog) {
1047 CastDialogModel model =
1048 CreateModelWithSinks({CreateAvailableSink()});
1049 InitializeDialogWithModel(model);
1050
1051 EXPECT_TRUE(dialog_->ShouldShowCloseButton());
1052 EXPECT_EQ(model.dialog_header(),
1053 dialog_->GetWindowTitle());
Md Hasibul Hasane914ad72024-08-20 12:38:581054 EXPECT_EQ(static_cast<int>(ui::mojom::DialogButton::kNone),
Wei Libc431812020-06-03 22:44:371055 dialog_->GetDialogButtons());
1056}
1057
1058
1059```
1060
1061#####
1062
1063``` cpp
1064void CastDialogView::PopulateScrollView(
1065 const std::vector<UIMediaSink>& sinks) {
1066 ...
1067 InvalidateLayout();
1068}
1069
1070TEST_F(CastDialogViewTest, PopulateDialog) {
1071 CastDialogModel model =
1072 CreateModelWithSinks({CreateAvailableSink()});
1073 InitializeDialogWithModel(model);
1074 CastDialogView::GetCurrentDialogWidget()
1075 ->LayoutRootViewIfNecessary();
1076
1077 EXPECT_TRUE(dialog_->ShouldShowCloseButton());
1078 EXPECT_EQ(model.dialog_header(),
1079 dialog_->GetWindowTitle());
Md Hasibul Hasane914ad72024-08-20 12:38:581080 EXPECT_EQ(static_cast<int>(ui::mojom::DialogButton::kNone),
Wei Libc431812020-06-03 22:44:371081 dialog_->GetDialogButtons());
1082}
1083```
1084
1085|||---|||
1086
1087
1088## Consider different objects for different layouts
1089
1090If a surface needs very different appearances in different states (e.g. a
1091dialog whose content changes at each of several steps, or a container whose
1092layout toggles between disparate orientations), **use different `View`s to
1093contain the distinct states** instead of manually adding and removing
1094children and changing layout properties at each step. It's easier to reason
1095about several distinct fixed-layout `View`s than a single object whose layout
1096and children vary over time, and often more performant as well.
1097
1098|||---|||
1099
1100#####
1101
1102**Avoid**
1103
1104[Current code][6] holds both horizontal and vertical time views and replaces
1105the children and LayoutManager on orientation change.
1106
John Palmer046f9872021-05-24 01:24:561107[6]: https://source.chromium.org/chromium/chromium/src/+/main:ash/system/time/time_view.h;l=35;drc=7d8bc7f807a433e6a127806e991fe780aa27ce77;bpv=1;bpt=0?originalUrl=https:%2F%2Fcs.chromium.org%2F
Wei Libc431812020-06-03 22:44:371108
1109#####
1110
1111**Best practice**
1112
1113A better approach would encapsulate the horizontal and vertical time views
1114into separate views.
1115
1116|||---|||
1117
1118|||---|||
1119
1120#####
1121
1122``` cpp
1123class ASH_EXPORT TimeView : public ActionableView,
1124 public ClockObserver {
1125 ...
1126 private:
1127 ...
1128 std::unique_ptr<views::Label> horizontal_label_;
1129 std::unique_ptr<views::Label> vertical_label_hours_;
1130 std::unique_ptr<views::Label> vertical_label_minutes_;
1131 ...
1132};
1133
1134
1135
1136
1137
1138
1139
1140
1141
1142
1143
1144
1145
1146
1147
1148
1149
1150
1151
1152
1153
1154
1155
1156
1157
1158
1159
1160
1161
1162
1163
1164
1165
1166
1167
1168
1169
1170
Wei Libc431812020-06-03 22:44:371171void TimeView::SetupLabels() {
1172 horizontal_label_.reset(new views::Label());
1173 SetupLabel(horizontal_label_.get());
1174 vertical_label_hours_.reset(new views::Label());
1175 SetupLabel(vertical_label_hours_.get());
1176 vertical_label_minutes_.reset(new views::Label());
1177 SetupLabel(vertical_label_minutes_.get());
1178 ...
1179}
1180
1181
1182
1183void TimeView::UpdateClockLayout(
1184 ClockLayout clock_layout) {
1185 // Do nothing if the layout hasn't changed.
1186 if (((clock_layout == ClockLayout::HORIZONTAL_CLOCK) ?
1187 horizontal_label_ : vertical_label_hours_)
1188 ->parent() == this)
1189 return;
1190
1191 SetBorder(views::NullBorder());
1192 if (clock_layout == ClockLayout::HORIZONTAL_CLOCK) {
1193 RemoveChildView(vertical_label_hours_.get());
1194 RemoveChildView(vertical_label_minutes_.get());
1195 SetLayoutManager(
1196 std::make_unique<views::FillLayout>());
1197 AddChildView(horizontal_label_.get());
1198 } else {
1199 RemoveChildView(horizontal_label_.get());
1200 // Remove the current layout manager since it could
1201 // be the FillLayout which only allows one child.
1202 SetLayoutManager(nullptr);
1203 // Pre-add the children since ownership is being
1204 // retained by this.
1205 AddChildView(vertical_label_hours_.get());
1206 AddChildView(vertical_label_minutes_.get());
1207 views::GridLayout* layout =
1208 SetLayoutManager(
1209 std::make_unique<views::GridLayout>());
1210 const int kColumnId = 0;
1211 views::ColumnSet* columns =
1212 layout->AddColumnSet(kColumnId);
1213 columns->AddPaddingColumn(
1214 0, kVerticalClockLeftPadding);
1215 columns->AddColumn(views::GridLayout::TRAILING,
1216 views::GridLayout::CENTER,
1217 0, views::GridLayout::USE_PREF,
1218 0, 0);
1219 layout->AddPaddingRow(0, kClockLeadingPadding);
1220 layout->StartRow(0, kColumnId);
1221 // Add the views as existing since ownership isn't
1222 // being transferred.
1223 layout->AddExistingView(
1224 vertical_label_hours_.get());
1225 layout->StartRow(0, kColumnId);
1226 layout->AddExistingView(
1227 vertical_label_minutes_.get());
1228 layout->AddPaddingRow(
1229 0, kVerticalClockMinutesTopOffset);
1230 }
Peter Kastingdf36f33e2024-01-31 19:21:341231 DeprecatedLayoutImmediately();
Wei Libc431812020-06-03 22:44:371232}
1233```
1234
1235#####
1236
1237``` cpp
1238class ASH_EXPORT TimeView : public ActionableView,
1239 public ClockObserver {
1240 ...
1241 private:
1242 class HorizontalLabelView;
1243 class VerticalLabelView;
1244 ...
1245 HorizontalLabelView* horizontal_label_;
1246 VerticalLabelView* vertical_label_;
1247 ...
1248};
1249
1250TimeView::HorizontalLabelView::HorizontalLabelView() {
1251 SetLayoutManager(
1252 std::make_unique<views::FillLayout>());
1253 views::Label* time_label =
1254 AddChildView(std::make_unique<views::Label>());
1255 SetupLabels(time_label);
1256 ...
1257}
1258
1259TimeView::VerticalLabelView::VerticalLabelView() {
Wei Libc431812020-06-03 22:44:371260 views::Label* label_hours =
1261 AddChildView(std::make_unique<views::Label>());
1262 views::Label* label_minutes =
1263 AddChildView(std::make_unique<views::Label>());
1264 SetupLabel(label_hours);
1265 SetupLabel(label_minutes);
Peter Kasting2ead0ee2021-10-20 16:55:271266 SetLayoutManager(
1267 std::make_unique<views::TableLayout>())
1268 ->AddPaddingColumn(
1269 views::TableLayout::kFixedSize,
1270 kVerticalClockLeftPadding)
1271 .AddColumn(
1272 views::LayoutAlignment::kEnd,
1273 views::LayoutAlignment::kCenter,
1274 views::TableLayout::kFixedSize,
1275 views::TableLayout::kUsePreferred, 0, 0)
1276 .AddPaddingRow(
1277 views::TableLayout::kFixedSize,
1278 kClockLeadingPadding)
1279 .AddRows(2, views::TableLayout::kFixedSize)
1280 .AddPaddingRow(
1281 views::TableLayout::kFixedSize,
1282 kVerticalClockMinutesTopOffset);
Wei Libc431812020-06-03 22:44:371283 ...
1284}
1285
1286void TimeView::TimeView(ClockLayout clock_layout,
1287 ClockModel* model) {
1288 ...
1289 horizontal_label_ =
1290 AddChildView(
1291 std::make_unique<HorizontalLabelView>());
1292 vertical_label_ =
1293 AddChildView(
Tom Sepez3fb65552022-12-01 21:33:491294 std::make_unique<VerticalLabelView>());
Wei Libc431812020-06-03 22:44:371295 ...
1296}
1297
1298void TimeView::UpdateClockLayout(
1299 ClockLayout clock_layout) {
1300 ...
1301 const bool is_horizontal =
1302 clock_layout == ClockLayout::HORIZONTAL_CLOCK;
1303 horizontal_label_->SetVisible(is_horizontal);
1304 vertical_label_->SetVisible(!is_horizontal);
1305 InvalidateLayout();
1306}
1307
1308
1309
1310
1311
1312
1313
1314
1315
1316
1317
1318
1319
1320
1321
1322
1323
1324
1325
1326
1327
1328
1329
1330
1331
1332
1333
1334
1335
1336
1337
1338
1339
1340
1341
1342
1343
1344
1345
1346
1347
1348```
1349
1350|||---|||