WebUI TaskManager: Remember column settings

WebUI TaskManager: Remember column settings

BUG=29950
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=125093

[yoshiki, 2012-03-01 09:25:53]

Could you take a look?

[James Hawkins, 2012-03-01 20:03:09]

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:648: if (checkedItemCount == 1 &&
checked) {
nit: No braces for single-line blocks.

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/preload.js (right):

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/preload.js:18: var cachedEnabledColumns;
Document variable.

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/preload.js:19: function
getEnabledColumns() {
Document method.

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/preload.js:37: function
isColumnEnabled(columnId) {
Document method.

http://codereview.chromium.org/9543002/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/preload.js:41: function
setEnabledColumns(enabledColumns) {
Document method.

[yoshiki, 2012-03-02 08:01:14]

Thanks! PTAL

https://chromiumcodereview.appspot.com/9543002/diff/1/chrome/browser/resource...
File chrome/browser/resources/task_manager/preload.js (right):

https://chromiumcodereview.appspot.com/9543002/diff/1/chrome/browser/resource...
chrome/browser/resources/task_manager/preload.js:18: var cachedEnabledColumns;
On 2012/03/01 20:03:09, James Hawkins wrote:
> Document variable.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/1/chrome/browser/resource...
chrome/browser/resources/task_manager/preload.js:19: function
getEnabledColumns() {
On 2012/03/01 20:03:09, James Hawkins wrote:
> Document method.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/1/chrome/browser/resource...
chrome/browser/resources/task_manager/preload.js:37: function
isColumnEnabled(columnId) {
On 2012/03/01 20:03:09, James Hawkins wrote:
> Document method.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/1/chrome/browser/resource...
chrome/browser/resources/task_manager/preload.js:41: function
setEnabledColumns(enabledColumns) {
On 2012/03/01 20:03:09, James Hawkins wrote:
> Document method.

Done.

[James Hawkins, 2012-03-02 22:52:38]

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
File chrome/browser/resources/task_manager/main.js (right):

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/main.js:654: setColumnEnablity(columnId,
newChecked);
Enablity is not a word, please find a better wording.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
File chrome/browser/resources/task_manager/preload.js (right):

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:18: // Caches list of enabled
columns to prevent frequent access to localStorage.
Variables don't "do" things, so s/Caches/Cached/.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:22: * Returns the dictionary
which maps columnId with enablity of column.
Same thing about enablity.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:43: * Returns whether the givin
column is enabled or not.
s/givin/given/

"Returns whether the given column (at |columnId|) is enabled or not."

[yoshiki, 2012-03-05 03:10:56]

Thanks, PTAL

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
File chrome/browser/resources/task_manager/main.js (right):

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/main.js:654: setColumnEnablity(columnId,
newChecked);
On 2012/03/02 22:52:38, James Hawkins wrote:
> Enablity is not a word, please find a better wording.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
File chrome/browser/resources/task_manager/preload.js (right):

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:18: // Caches list of enabled
columns to prevent frequent access to localStorage.
On 2012/03/02 22:52:38, James Hawkins wrote:
> Variables don't "do" things, so s/Caches/Cached/.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:22: * Returns the dictionary
which maps columnId with enablity of column.
On 2012/03/02 22:52:38, James Hawkins wrote:
> Same thing about enablity.

Done.

https://chromiumcodereview.appspot.com/9543002/diff/5001/chrome/browser/resou...
chrome/browser/resources/task_manager/preload.js:43: * Returns whether the givin
column is enabled or not.
On 2012/03/02 22:52:38, James Hawkins wrote:
> s/givin/given/
> 
> "Returns whether the given column (at |columnId|) is enabled or not."

Done.

[James Hawkins, 2012-03-06 01:33:24]

LGTM, thanks!

[commit-bot: I haz the power, 2012-03-06 01:37:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/9543002/8001

[commit-bot: I haz the power, 2012-03-06 03:32:32]

Change committed as 125093