Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issue 8555009: Fix display and progressive disclosure for multi-line values in about:policy. (Closed)

Created:
9 years, 1 month ago by Mattias Nissler (ping if slow)
Modified:
9 years, 1 month ago
CC:
chromium-reviews, arv (Not doing code reviews)
Visibility:
Public.

Description

Fix display and progressive disclosure for multi-line values in about:policy. Previously, all lines were joined together into a single hard-to-read block. Change expanded state to make white-space significant so multi-line error messages are on separate lines and network configuration renders nicely. BUG=chromium:104138 TEST=Put network configuration policy in place and check about:policy. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110070

Patch Set 1 #

Total comments: 6

Patch Set 2 : Address comments, make hide/show proper links. #

Total comments: 4

Patch Set 3 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -75 lines) Patch
M chrome/app/generated_resources.grd View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/policy/policy_error_map.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/policy.css View 1 2 1 chunk +28 lines, -8 lines 0 comments Download
M chrome/browser/resources/policy.html View 1 2 1 chunk +16 lines, -14 lines 0 comments Download
M chrome/browser/resources/policy.js View 1 6 chunks +41 lines, -50 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Mattias Nissler (ping if slow)
James, can you please review the HTML/CSS? Joao, just a single chrome/browser/policy line for you ...
9 years, 1 month ago (2011-11-14 16:12:37 UTC) #1
Joao da Silva
policy LGTM
9 years, 1 month ago (2011-11-14 16:47:02 UTC) #2
James Hawkins
http://codereview.chromium.org/8555009/diff/1/chrome/browser/resources/policy.html File chrome/browser/resources/policy.html (right): http://codereview.chromium.org/8555009/diff/1/chrome/browser/resources/policy.html#newcode131 chrome/browser/resources/policy.html:131: <a class="toggler expand" style="display: none" Use hidden instead of ...
9 years, 1 month ago (2011-11-14 17:34:47 UTC) #3
Mattias Nissler (ping if slow)
PTAL. Thanks! http://codereview.chromium.org/8555009/diff/1/chrome/browser/resources/policy.html File chrome/browser/resources/policy.html (right): http://codereview.chromium.org/8555009/diff/1/chrome/browser/resources/policy.html#newcode131 chrome/browser/resources/policy.html:131: <a class="toggler expand" style="display: none" On 2011/11/14 ...
9 years, 1 month ago (2011-11-14 17:59:28 UTC) #4
James Hawkins
http://codereview.chromium.org/8555009/diff/2003/chrome/browser/resources/policy.html File chrome/browser/resources/policy.html (right): http://codereview.chromium.org/8555009/diff/2003/chrome/browser/resources/policy.html#newcode131 chrome/browser/resources/policy.html:131: <a class="link-button toggler expand" hidden You shouldn't use both ...
9 years, 1 month ago (2011-11-14 18:12:35 UTC) #5
Mattias Nissler (ping if slow)
And another try... http://codereview.chromium.org/8555009/diff/2003/chrome/browser/resources/policy.html File chrome/browser/resources/policy.html (right): http://codereview.chromium.org/8555009/diff/2003/chrome/browser/resources/policy.html#newcode131 chrome/browser/resources/policy.html:131: <a class="link-button toggler expand" hidden On ...
9 years, 1 month ago (2011-11-14 18:36:17 UTC) #6
James Hawkins
lgtm
9 years, 1 month ago (2011-11-14 19:17:12 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mnissler@chromium.org/8555009/5002
9 years, 1 month ago (2011-11-15 09:47:18 UTC) #8
commit-bot: I haz the power
9 years, 1 month ago (2011-11-15 11:27:03 UTC) #9
Change committed as 110070

Powered by Google App Engine
This is Rietveld 408576698