Issue 6596064: UI for 802.1x connections.

Issue 6596064: UI for 802.1x connections. (Closed)

Created:
9 years, 9 months ago by Charlie Lee

Modified:
9 years, 7 months ago

Reviewers:
stevenjb

CC:
chromium-reviews, davemoore+watch_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

UI for 802.1x connections. BUG=chromium-os:11412 TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76865

Patch Set 4 : '' #

Total comments: 1

Created: 9 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+318 lines, -94 lines)			Patch
M	chrome/app/generated_resources.grd	View	1 2 3	1 chunk	+45 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/options/network_config_view.cc	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/chromeos/options/wifi_config_view.h	View	1 2 3	3 chunks	+9 lines, -15 lines	0 comments	Download
M	chrome/browser/chromeos/options/wifi_config_view.cc	View	1 2 3	10 chunks	+263 lines, -78 lines	1 comment	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Charlie Lee

http://codereview.chromium.org/6596064/diff/9001/chrome/browser/chromeos/options/wifi_config_view.cc File chrome/browser/chromeos/options/wifi_config_view.cc (right): http://codereview.chromium.org/6596064/diff/9001/chrome/browser/chromeos/options/wifi_config_view.cc#newcode469 chrome/browser/chromeos/options/wifi_config_view.cc:469: if (is_8021x_) { This code is actually not live ...

9 years, 9 months ago (2011-03-04 01:27:23 UTC) #1

stevenjb

LGTM with one question/caveat. http://codereview.chromium.org/6596064/diff/9001/chrome/browser/chromeos/options/wifi_config_view.h File chrome/browser/chromeos/options/wifi_config_view.h (right): http://codereview.chromium.org/6596064/diff/9001/chrome/browser/chromeos/options/wifi_config_view.h#newcode107 chrome/browser/chromeos/options/wifi_config_view.h:107: }; Do these need to ...

9 years, 9 months ago (2011-03-04 01:45:53 UTC) #2

Charlie Lee

If I don't hear back from you later, I will check this in. Thanks! http://codereview.chromium.org/6596064/diff/9001/chrome/browser/chromeos/options/wifi_config_view.h ...

9 years, 9 months ago (2011-03-04 01:54:14 UTC) #3

stevenjb

9 years, 9 months ago (2011-03-04 03:21:02 UTC) #4

Was able to take a longer look on the bus. Still LGTM (definitely like the combo
classes in the .cc file), with a tiny possible nit. Go ahead and push with or
without that change.

http://codereview.chromium.org/6596064/diff/11002/chrome/browser/chromeos/opt...
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

http://codereview.chromium.org/6596064/diff/11002/chrome/browser/chromeos/opt...
chrome/browser/chromeos/options/wifi_config_view.cc:236: void
WifiConfigView::UpdateCanLogin() {
nit: rename this to something like UpdateDialog()?

Expand Messages | Collapse Messages