Issue 9875001: views: Rename Combobox "selected getter/setter" accessors to something more accurate.

Issue 9875001: views: Rename Combobox "selected getter/setter" accessors to something more accurate. (Closed)

Created:
8 years, 9 months ago by tfarina

Modified:
8 years, 9 months ago

Reviewers:
sky

CC:
chromium-reviews, tfarina, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

views: Rename Combobox "selected getter/setter" accessors to something more accurate. Currently the accessors are named as selected_item() and SetSelectedItem(). But we don't return any "item" at all, nor we set any "item". They aren't objects. They are indexes. So a more accurate name would be selected_index() and SetSelectedIndex(). This match with other ComboBox implementations (Java and .Net). BUG=119289 R=sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129464

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+167 lines, -175 lines)			Patch
M	chrome/browser/chromeos/options/vpn_config_view.cc	View		5 chunks	+17 lines, -18 lines	0 comments	Download
M	chrome/browser/chromeos/options/wifi_config_view.cc	View		19 chunks	+47 lines, -53 lines	0 comments	Download
M	chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/views/cookie_info_view.cc	View		2 chunks	+7 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/views/uninstall_view.cc	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	ui/views/controls/combobox/combobox.h	View	1 2	2 chunks	+14 lines, -15 lines	0 comments	Download
M	ui/views/controls/combobox/combobox.cc	View	1	4 chunks	+12 lines, -11 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_views.h	View	1	3 chunks	+6 lines, -5 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_views.cc	View	1	5 chunks	+25 lines, -27 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_views_unittest.cc	View	1 2	2 chunks	+6 lines, -6 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_win.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_win.cc	View	1 2	2 chunks	+8 lines, -8 lines	0 comments	Download
M	ui/views/controls/combobox/native_combobox_wrapper.h	View	1 2	2 chunks	+10 lines, -11 lines	0 comments	Download
M	ui/views/examples/combobox_example.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	ui/views/examples/native_theme_button_example.cc	View		4 chunks	+5 lines, -7 lines	0 comments	Download
M	ui/views/examples/text_example.h	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	ui/views/examples/text_example.cc	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

sky

8 years, 9 months ago (2012-03-27 22:52:32 UTC) #2

LGTM

http://codereview.chromium.org/9875001/diff/4004/ui/views/controls/combobox/c...
File ui/views/controls/combobox/combobox.h (right):

http://codereview.chromium.org/9875001/diff/4004/ui/views/controls/combobox/c...
ui/views/controls/combobox/combobox.h:27: // A non-editable combo box (aka a
drop-down list).
Because the class is named 'Combobox' we should use combobox every where.

http://codereview.chromium.org/9875001/diff/4004/ui/views/controls/combobox/c...
ui/views/controls/combobox/combobox.h:99: // The accessible name of this combo
box.
combobox

Expand Messages | Collapse Messages