Supervised users: UI updates for new-profile-management launch.

Supervised users: UI updates for new-profile-management launch, as decided in UI review.

- Remove "of this user" from the supervision disclaimer.

In the profile list in about:settings:
- Change the SU label from "Supervised user" to "Supervised"

In the create profile overlay:
- Hide the "import existing supervised user" link if the current user is signed in, but has no supervised users.
- Remove "sign in to import SUs" label.
- Change strings.

BUG=400744, 403290, 403293, 403295
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289576

[Marc Treib, 2014-08-06 13:48:37]

PTAL!

[Pam (message me for reviews), 2014-08-06 23:36:36]

https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/manage_profile_overlay.js (right):

https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/manage_profile_overlay.js:819: if (!enable)
This check is backwards, and it's not quite what we want anyway. In practice
this method is called when the signin state changes. If the current profile is
not signed in, in principle we don't know who it is, so we should treat it as
though it has no SUs and hide the link.

But that makes this method confusing. I'd suggest changing the parameter name to
"isSignedIn", and then we can make the treatment of each property clearer.

[Marc Treib, 2014-08-07 07:39:34]

https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/manage_profile_overlay.js (right):

https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/manage_profile_overlay.js:819: if (!enable)
On 2014/08/06 23:36:36, Pam (also PM for reviews) wrote:
> This check is backwards, and it's not quite what we want anyway. In practice
> this method is called when the signin state changes. If the current profile is
> not signed in, in principle we don't know who it is, so we should treat it as
> though it has no SUs and hide the link.
> 
> But that makes this method confusing. I'd suggest changing the parameter name
to
> "isSignedIn", and then we can make the treatment of each property clearer.

When we're not signed in, the link is disabled (i.e. becomes a label) and its
text is changed to "sign in to import SUs". That means we would want to show the
"link" when either we are not signed in, or we are signed in and have SUs. This
is what makes the whole thing so confusing: If we are signed in, then at this
point we don't know whether to show or hide the link.
I can see two ways of fixing that:
1) Just hide the "link"/label when we're not signed in. It's a bit redundant
anyway, since there's also a label saying "sign in to *create* SUs".
2) Make separate divs for the link and the label, which will make it clear what
is being shown/hidden.

[Pam (message me for reviews), 2014-08-12 19:36:08]

On 2014/08/07 07:39:34, Marc Treib wrote:
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> File chrome/browser/resources/options/manage_profile_overlay.js (right):
> 
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> chrome/browser/resources/options/manage_profile_overlay.js:819: if (!enable)
> On 2014/08/06 23:36:36, Pam (also PM for reviews) wrote:
> > This check is backwards, and it's not quite what we want anyway. In practice
> > this method is called when the signin state changes. If the current profile
is
> > not signed in, in principle we don't know who it is, so we should treat it
as
> > though it has no SUs and hide the link.
> > 
> > But that makes this method confusing. I'd suggest changing the parameter
name
> to
> > "isSignedIn", and then we can make the treatment of each property clearer.
> 
> When we're not signed in, the link is disabled (i.e. becomes a label) and its
> text is changed to "sign in to import SUs". That means we would want to show
the
> "link" when either we are not signed in, or we are signed in and have SUs.
This
> is what makes the whole thing so confusing: If we are signed in, then at this
> point we don't know whether to show or hide the link.
> I can see two ways of fixing that:
> 1) Just hide the "link"/label when we're not signed in. It's a bit redundant
> anyway, since there's also a label saying "sign in to *create* SUs".
> 2) Make separate divs for the link and the label, which will make it clear
what
> is being shown/hidden.

What do you think about changing the text on the disabled creation checkbox to
"Sign in to create or import SUs", and then hiding the redundant, disabled
import label?

[Marc Treib, 2014-08-13 08:32:56]

On 2014/08/12 19:36:08, Pam (also PM for reviews) wrote:
> On 2014/08/07 07:39:34, Marc Treib wrote:
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > File chrome/browser/resources/options/manage_profile_overlay.js (right):
> > 
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > chrome/browser/resources/options/manage_profile_overlay.js:819: if (!enable)
> > On 2014/08/06 23:36:36, Pam (also PM for reviews) wrote:
> > > This check is backwards, and it's not quite what we want anyway. In
practice
> > > this method is called when the signin state changes. If the current
profile
> is
> > > not signed in, in principle we don't know who it is, so we should treat it
> as
> > > though it has no SUs and hide the link.
> > > 
> > > But that makes this method confusing. I'd suggest changing the parameter
> name
> > to
> > > "isSignedIn", and then we can make the treatment of each property clearer.
> > 
> > When we're not signed in, the link is disabled (i.e. becomes a label) and
its
> > text is changed to "sign in to import SUs". That means we would want to show
> the
> > "link" when either we are not signed in, or we are signed in and have SUs.
> This
> > is what makes the whole thing so confusing: If we are signed in, then at
this
> > point we don't know whether to show or hide the link.
> > I can see two ways of fixing that:
> > 1) Just hide the "link"/label when we're not signed in. It's a bit redundant
> > anyway, since there's also a label saying "sign in to *create* SUs".
> > 2) Make separate divs for the link and the label, which will make it clear
> what
> > is being shown/hidden.
> 
> What do you think about changing the text on the disabled creation checkbox to
> "Sign in to create or import SUs", and then hiding the redundant, disabled
> import label?

SGTM, and that's also more or less what the Mirror/UX people seem to have
converged on in the "Implementation Review" email thread. I'll do that, and
repurpose this CL to include the other string changes as well.

[Marc Treib, 2014-08-13 11:04:03]

On 2014/08/13 08:32:56, Marc Treib wrote:
> On 2014/08/12 19:36:08, Pam (also PM for reviews) wrote:
> > On 2014/08/07 07:39:34, Marc Treib wrote:
> > >
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > > File chrome/browser/resources/options/manage_profile_overlay.js (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > > chrome/browser/resources/options/manage_profile_overlay.js:819: if
(!enable)
> > > On 2014/08/06 23:36:36, Pam (also PM for reviews) wrote:
> > > > This check is backwards, and it's not quite what we want anyway. In
> practice
> > > > this method is called when the signin state changes. If the current
> profile
> > is
> > > > not signed in, in principle we don't know who it is, so we should treat
it
> > as
> > > > though it has no SUs and hide the link.
> > > > 
> > > > But that makes this method confusing. I'd suggest changing the parameter
> > name
> > > to
> > > > "isSignedIn", and then we can make the treatment of each property
clearer.
> > > 
> > > When we're not signed in, the link is disabled (i.e. becomes a label) and
> its
> > > text is changed to "sign in to import SUs". That means we would want to
show
> > the
> > > "link" when either we are not signed in, or we are signed in and have SUs.
> > This
> > > is what makes the whole thing so confusing: If we are signed in, then at
> this
> > > point we don't know whether to show or hide the link.
> > > I can see two ways of fixing that:
> > > 1) Just hide the "link"/label when we're not signed in. It's a bit
redundant
> > > anyway, since there's also a label saying "sign in to *create* SUs".
> > > 2) Make separate divs for the link and the label, which will make it clear
> > what
> > > is being shown/hidden.
> > 
> > What do you think about changing the text on the disabled creation checkbox
to
> > "Sign in to create or import SUs", and then hiding the redundant, disabled
> > import label?
> 
> SGTM, and that's also more or less what the Mirror/UX people seem to have
> converged on in the "Implementation Review" email thread. I'll do that, and
> repurpose this CL to include the other string changes as well.

Okay, I think the functionality is done. The strings are probably not quite
final, as the discussion is still ongoing. Maybe you can already preemptively
lgtm this? Patrick would like to land it this week (and Friday's a public
holiday).

[Marc Treib, 2014-08-13 12:52:42]

On 2014/08/13 11:04:03, Marc Treib wrote:
> On 2014/08/13 08:32:56, Marc Treib wrote:
> > On 2014/08/12 19:36:08, Pam (also PM for reviews) wrote:
> > > On 2014/08/07 07:39:34, Marc Treib wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > > > File chrome/browser/resources/options/manage_profile_overlay.js (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/445003002/diff/1/chrome/browser/resources/opt...
> > > > chrome/browser/resources/options/manage_profile_overlay.js:819: if
> (!enable)
> > > > On 2014/08/06 23:36:36, Pam (also PM for reviews) wrote:
> > > > > This check is backwards, and it's not quite what we want anyway. In
> > practice
> > > > > this method is called when the signin state changes. If the current
> > profile
> > > is
> > > > > not signed in, in principle we don't know who it is, so we should
treat
> it
> > > as
> > > > > though it has no SUs and hide the link.
> > > > > 
> > > > > But that makes this method confusing. I'd suggest changing the
parameter
> > > name
> > > > to
> > > > > "isSignedIn", and then we can make the treatment of each property
> clearer.
> > > > 
> > > > When we're not signed in, the link is disabled (i.e. becomes a label)
and
> > its
> > > > text is changed to "sign in to import SUs". That means we would want to
> show
> > > the
> > > > "link" when either we are not signed in, or we are signed in and have
SUs.
> > > This
> > > > is what makes the whole thing so confusing: If we are signed in, then at
> > this
> > > > point we don't know whether to show or hide the link.
> > > > I can see two ways of fixing that:
> > > > 1) Just hide the "link"/label when we're not signed in. It's a bit
> redundant
> > > > anyway, since there's also a label saying "sign in to *create* SUs".
> > > > 2) Make separate divs for the link and the label, which will make it
clear
> > > what
> > > > is being shown/hidden.
> > > 
> > > What do you think about changing the text on the disabled creation
checkbox
> to
> > > "Sign in to create or import SUs", and then hiding the redundant, disabled
> > > import label?
> > 
> > SGTM, and that's also more or less what the Mirror/UX people seem to have
> > converged on in the "Implementation Review" email thread. I'll do that, and
> > repurpose this CL to include the other string changes as well.
> 
> Okay, I think the functionality is done. The strings are probably not quite
> final, as the discussion is still ongoing. Maybe you can already preemptively
> lgtm this? Patrick would like to land it this week (and Friday's a public
> holiday).

...aaand now the strings are actually "probably" final. PTAL!

[Pam (message me for reviews), 2014-08-13 14:25:47]

LGTM with the suggested change.

https://codereview.chromium.org/445003002/diff/80001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/445003002/diff/80001/chrome/app/generated_res...
chrome/app/generated_resources.grd:13272: +        <ph
name="BEGIN_SIGN_IN_LINK">&lt;button id="create-profile-supervised-sign-in-link"
class="link-button"&gt;</ph>Sign in<ph
name="END_SIGN_IN_LINK">&lt;/button&gt;</ph> to control and view the websites
this person visits.
I'm reluctant to mix element ID and class directly into the strings, giving one
more place people have to check when they update the CSS. But I see that it's
been done before (very rarely), and it is a cleaner structure, so OK.

https://codereview.chromium.org/445003002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/options/manage_profile_overlay.js (right):

https://codereview.chromium.org/445003002/diff/80001/chrome/browser/resources...
chrome/browser/resources/options/manage_profile_overlay.js:93:
loadTimeData.getStringF('manageProfilesSupervisedNotSignedIn');
You can do this directly in the HTML file, using
  i18n-values=".innerHTML:manageProfilesSupervisedNotSignedIn"

[Marc Treib, 2014-08-13 16:08:45]

https://codereview.chromium.org/445003002/diff/80001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/445003002/diff/80001/chrome/app/generated_res...
chrome/app/generated_resources.grd:13272: +        <ph
name="BEGIN_SIGN_IN_LINK">&lt;button id="create-profile-supervised-sign-in-link"
class="link-button"&gt;</ph>Sign in<ph
name="END_SIGN_IN_LINK">&lt;/button&gt;</ph> to control and view the websites
this person visits.
On 2014/08/13 14:25:47, Pam (also PM for reviews) wrote:
> I'm reluctant to mix element ID and class directly into the strings, giving
one
> more place people have to check when they update the CSS. But I see that it's
> been done before (very rarely), and it is a cleaner structure, so OK.

Yes, I very much dislike this as well. The reason I did it like this is for
translation: The "Sign in" link might not be at the beginning of the sentence in
all languages.

https://codereview.chromium.org/445003002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/options/manage_profile_overlay.js (right):

https://codereview.chromium.org/445003002/diff/80001/chrome/browser/resources...
chrome/browser/resources/options/manage_profile_overlay.js:93:
loadTimeData.getStringF('manageProfilesSupervisedNotSignedIn');
On 2014/08/13 14:25:47, Pam (also PM for reviews) wrote:
> You can do this directly in the HTML file, using
>   i18n-values=".innerHTML:manageProfilesSupervisedNotSignedIn"

Ah, nice! TIL :)

[Marc Treib, 2014-08-13 16:08:52]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-08-13 16:10:07]

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

[commit-bot: I haz the power, 2014-08-13 19:55:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-13 21:13:25]

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

[commit-bot: I haz the power, 2014-08-13 21:13:26]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Marc Treib, 2014-08-14 07:45:07]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-08-14 07:48:57]

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

[commit-bot: I haz the power, 2014-08-14 15:01:14]

Committed patchset #7 (120001) as 289576