Simplify usage of dialog metrics to always use insets |
||||
Issue descriptionFrom review comments on a CL: "Here's a thought for a path forward: (1) Provide INSETS_DIALOG, which is the dialog's outer insets. INSETS_DIALOG_XXX for the sub-pieces can stick around for convenience if we want, and are computed by taking INSETS_DIALOG and then setting the top or bottom or whatever to zero. (To do this, it would be convenient if Insets had setters fro individual sides. I've wanted those a lot in the past weeks.) (2) Make BoxLayout take an Insets arg, instead of the horizontal and vertical constants it has now. This is more powerful and simpler to specify, and we can replace callsites like the ones in this CL with just passing INSETS_DIALOG. (3) Once this is done, look to eliminate direct usage of DISTANCE_DIALOG_CONTENTS_XXX_MARGIN in favor of having people use the insets metrics above. This aims to force callers not to assume that the left and right sides match, for example. (4) Once that's done, make DISTANCE_DIALOG_CONTENTS_XXX_MARGIN be protected in layout_provider.h instead of public, so they become "implementation detail" metrics. In my head, this makes the system clearer and simpler, yet also slightly more powerful than before."
,
Jun 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f135c9f1fc53033408152e5024c1fe30c3bb3bb commit 7f135c9f1fc53033408152e5024c1fe30c3bb3bb Author: Allen Bauer <kylixrd@chromium.org> Date: Wed Jun 07 15:56:16 2017 use INSETS_DIALOG_CONTENTS instead of the individual DISTANCE_XXX metrics. Bug: 723924 Change-Id: Ia0bb3b9aa1bd5d759ae515a8e89a0224fb479e27 Reviewed-on: https://chromium-review.googlesource.com/526279 Commit-Queue: Allen Bauer <kylixrd@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#477664} [modify] https://crrev.com/7f135c9f1fc53033408152e5024c1fe30c3bb3bb/chrome/browser/ui/views/importer/import_lock_dialog_view.cc [modify] https://crrev.com/7f135c9f1fc53033408152e5024c1fe30c3bb3bb/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc [modify] https://crrev.com/7f135c9f1fc53033408152e5024c1fe30c3bb3bb/chrome/browser/ui/views/profiles/profile_chooser_view.cc
,
Jun 14 2017
All but (4) is done. Not sure how to go about making a single element of an enumeration protected.
,
Jun 14 2017
We decided to be less clever about (4) and, instead of using protected values, just rip out DISTANCE_DIALOG_CONTENTS_XXX_MARGIN and override the insets in the layout provider subclasses. This is almost ready to do pending a couple remaining cases of (3) regarding the vertical margin.
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ef14c385edc28a88c08207698acdccdee33810f commit 6ef14c385edc28a88c08207698acdccdee33810f Author: Allen Bauer <kylixrd@chromium.org> Date: Wed Jun 14 19:04:52 2017 Use DISTANCE_UNRELATED_CONTROL_HORIZONTAL instead of INSETS_DIALOG_CONTENTS. Bug: 723924 Change-Id: I0f209dd81b59b820ac3f92cfdd954513530ecef9 Reviewed-on: https://chromium-review.googlesource.com/527998 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Allen Bauer <kylixrd@chromium.org> Cr-Commit-Position: refs/heads/master@{#479459} [modify] https://crrev.com/6ef14c385edc28a88c08207698acdccdee33810f/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc [modify] https://crrev.com/6ef14c385edc28a88c08207698acdccdee33810f/chrome/browser/ui/views/profiles/profile_chooser_view.cc
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208 commit 19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208 Author: Allen Bauer <kylixrd@chromium.org> Date: Wed Jun 21 13:00:14 2017 Removed final use of DISTANCE_DIALOG_XXXXX_XXXX_MARGIN. Bug: 723924 Change-Id: I57820e64629fe6156404266806dba18d32f0c729 Reviewed-on: https://chromium-review.googlesource.com/537175 Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Allen Bauer <kylixrd@chromium.org> Cr-Commit-Position: refs/heads/master@{#481185} [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/browser/ui/views/extensions/media_galleries_dialog_views.cc [add] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/browser/ui/views/extensions/media_galleries_dialog_views_browsertest.cc [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/browser/ui/views/extensions/media_gallery_checkbox_view.cc [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/browser/ui/views/harmony/harmony_layout_provider.cc [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/browser/ui/views/page_info/page_info_bubble_view.cc [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/chrome/test/BUILD.gn [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/ui/views/layout/layout_provider.cc [modify] https://crrev.com/19d2c5cb7b75b0c04153fbeff487ab8e8fcfa208/ui/views/layout/layout_provider.h
,
Jun 21 2017
All uses of DISTANCE_DIALOG_XXXX and DISTANCE_BUBBLE_XXXX changed to use INSETS_DIALOG_CONTENTS and INSETS_BUBBLE_CONTENTS and enum elements deleted. |
||||
►
Sign in to add a comment |
||||
Comment 1 by pkasting@chromium.org
, May 23 2017