New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 854186 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Magic constants in GridLayout

Project Member Reported by osc...@opera.com, Jun 19 2018

Issue description

The functions:

* AddPaddingRow
* StartRowWithPadding
* StartRow
* AddPaddingColumn
* AddColumn

in GridLayout (chromium/src/ui/views/grid_layout.h) receives a parameter that defines how much the view should scale. The parameter is a float and is almost every time passed as 0 or 1, but there are cases where it is 0.5 or 100. This parameter is sometimes passed as variable, locally defined as:

constexpr float kStretchy = 1.f
constexpr float kFixed = 0.f

but most often as a magic constant.

The inconsistent usage makes it hard to understand how the parameter should be used and what is does. This could be solved by defining kStretchy and kFixed in the namespace views::GridLayout and use it instead of magic constants.

Patch incoming.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa

commit b227cc09b41fed93dcb32fe1e2feff3341cbe1aa
Author: Oscar Johansson <oscarj@opera.com>
Date: Wed Jun 27 08:10:36 2018

Replace magic constant with kFixedSize

When calling certain functions in GridLayout a resize
parameter is passed, defining how much the layout should
resize.
Most often it is passed as 0 or 1. Sometimes 100 (which
is assumed to be the same as 1) and sometimes 0.5.

This commit defines the variable kFixedSize = 0.f
in grid_layout.h and remove every local defintion of
the variables. The magic constant are replaced by this
variable. The replacements are:
0, 0.f and 0.0f becomes kFixedSize (0.f)

This commit affects chrome/browser/ui.

Bug:  854186 
Change-Id: I287b1b38350a2dce2bd6e02cc871e82e0a5d89db
Reviewed-on: https://chromium-review.googlesource.com/1106159
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Oscar Johansson <oscarj@opera.com>
Cr-Commit-Position: refs/heads/master@{#570693}
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/ash/network/enrollment_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/certificate_selector.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/confirm_bubble_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/content_setting_bubble_contents.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/cookie_info_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/create_application_shortcut_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/crypto_module_password_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/extensions/media_galleries_dialog_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/first_run_dialog.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/global_error_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/harmony/bulleted_label_list_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/harmony/textfield_layout.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/hover_button.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/hung_renderer_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/ime/ime_warning_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/intent_picker_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/login_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/page_info/chosen_object_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/page_info/permission_selector_row.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/passwords/password_generation_popup_view_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/passwords/password_items_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/passwords/password_pending_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/editor_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/order_summary_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_request_item_list.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_request_sheet_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_request_views_util.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/policy/enterprise_startup_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/profiles/forced_reauthentication_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/profiles/profile_chooser_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/proximity_auth/proximity_auth_error_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/session_crashed_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/sync/dice_signin_button_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/sync/one_click_signin_dialog_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/try_chrome_dialog_win/try_chrome_dialog.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/chrome/browser/ui/views/uninstall_view.cc
[modify] https://crrev.com/b227cc09b41fed93dcb32fe1e2feff3341cbe1aa/ui/views/layout/grid_layout.h

Status: Fixed (was: Started)
kFixedSize is fixed.

Sign in to add a comment