Mark supervised users in the "Users" list in about:settings.

Mark supervised users in the "Users" list in about:settings.

BUG=107602
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261391

[Patrick Dubroy, 2014-04-02 14:42:05]

https://codereview.chromium.org/222143002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:91: -webkit-box-flex: 1;
We shouldn't mix and match different flexbox specs here.

[Marc Treib, 2014-04-03 10:13:32]

https://codereview.chromium.org/222143002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:91: -webkit-box-flex: 1;
On 2014/04/02 14:42:05, Patrick Dubroy wrote:
> We shouldn't mix and match different flexbox specs here.

Done.

[Patrick Dubroy, 2014-04-03 10:48:38]

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:94: white-space: nowrap;
I don't think you need this.

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:105: color: gray;
I don't see "gray" anywhere else in this file. Usually we use rgb or hex values
for colors...is there another element on this page with the same color that you
could be consistent with?

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:107: white-space: nowrap;
Ditto.

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options_profile_list.js (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options_profile_list.js:56:
containerEl.className = 'profile-container';
Could you just give this class to `this.contentElement` instead, and avoid the
extra level of nesting?

[Marc Treib, 2014-04-03 11:41:56]

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:94: white-space: nowrap;
On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> I don't think you need this.

Done.

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:105: color: gray;
On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> I don't see "gray" anywhere else in this file. Usually we use rgb or hex
values
> for colors...is there another element on this page with the same color that
you
> could be consistent with?

I don't see another "gray for de-emphasis" anywhere else on the page, and
reusing a "gray for disabled" color doesn't make much sense. I could just
replace it by, say, #777 if named colors are discouraged in general? ("white"
and "black" do appear in this file, though!)

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:107: white-space: nowrap;
On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> Ditto.

Done.

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options_profile_list.js (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options_profile_list.js:56:
containerEl.className = 'profile-container';
On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> Could you just give this class to `this.contentElement` instead, and avoid the
> extra level of nesting?

No: this.contentElement will not shrink, and so the "(Supervised user)" will
appear right-aligned instead of left. That could probably be fixed by overriding
some of this.contentElement's pre-defined properties, but at that point the
extra nesting seems cleaner to me.

[Patrick Dubroy, 2014-04-03 11:48:37]

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:105: color: gray;
On 2014/04/03 11:41:56, treib wrote:
> On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> > I don't see "gray" anywhere else in this file. Usually we use rgb or hex
> values
> > for colors...is there another element on this page with the same color that
> you
> > could be consistent with?
> 
> I don't see another "gray for de-emphasis" anywhere else on the page, and
> reusing a "gray for disabled" color doesn't make much sense. I could just
> replace it by, say, #777 if named colors are discouraged in general? ("white"
> and "black" do appear in this file, though!)

Actually, we seem to use #999 in a few places (e.g., policy-controlled
settings), and in the sidebar menu. Maybe this should be the same?

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options_profile_list.js (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options_profile_list.js:56:
containerEl.className = 'profile-container';
On 2014/04/03 11:41:56, treib wrote:
> On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> > Could you just give this class to `this.contentElement` instead, and avoid
the
> > extra level of nesting?
> 
> No: this.contentElement will not shrink, and so the "(Supervised user)" will
> appear right-aligned instead of left. That could probably be fixed by
overriding
> some of this.contentElement's pre-defined properties, but at that point the
> extra nesting seems cleaner to me.

Ok.

[Marc Treib, 2014-04-03 12:01:08]

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/222143002/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.css:105: color: gray;
On 2014/04/03 11:48:38, Patrick Dubroy wrote:
> On 2014/04/03 11:41:56, treib wrote:
> > On 2014/04/03 10:48:39, Patrick Dubroy wrote:
> > > I don't see "gray" anywhere else in this file. Usually we use rgb or hex
> > values
> > > for colors...is there another element on this page with the same color
that
> > you
> > > could be consistent with?
> > 
> > I don't see another "gray for de-emphasis" anywhere else on the page, and
> > reusing a "gray for disabled" color doesn't make much sense. I could just
> > replace it by, say, #777 if named colors are discouraged in general?
("white"
> > and "black" do appear in this file, though!)
> 
> Actually, we seem to use #999 in a few places (e.g., policy-controlled
> settings), and in the sidebar menu. Maybe this should be the same?

Done.

[Patrick Dubroy, 2014-04-03 12:13:56]

lgtm

[Marc Treib, 2014-04-03 12:19:30]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-04-03 12:19:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/treib@chromium.org/222143002/100001

[commit-bot: I haz the power, 2014-04-03 13:14:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 13:14:43]

Retried try job too often on ios_rel_device_ninja for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_rel_de...

[Marc Treib, 2014-04-03 14:20:56]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-04-03 14:21:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/treib@chromium.org/222143002/100001

[commit-bot: I haz the power, 2014-04-03 14:59:50]

Change committed as 261391