MD User Manager: fixes accessibility issues

MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/1aa2dbfa94ccc24698fe5ffd13573a5f68cea0d3
Cr-Commit-Position: refs/heads/master@{#392053}

[Moe, 2016-05-02 22:09:42]

Description was changed from

==========
MD User Manager: fixes accessibility issues

Signed-off-by: mahmadi <mahmadi@chromium.org>

BUG=
==========

to

==========
MD User Manager: fixes accessibility issues

Signed-off-by: mahmadi <mahmadi@chromium.org>

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Moe, 2016-05-02 22:16:08]

Description was changed from

==========
MD User Manager: fixes accessibility issues

Signed-off-by: mahmadi <mahmadi@chromium.org>

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Moe, 2016-05-02 22:16:09]

mahmadi@chromium.org changed reviewers:
+ tommycli@chromium.org

[Moe, 2016-05-02 22:16:29]

Tommy, please review this patch. Thank you!

[Moe, 2016-05-03 21:45:55]

Description was changed from

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tommycli, 2016-05-04 17:04:11]

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/import_supervised_user.html
(right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/import_supervised_user.html:66:
font-size: inherit;
Are these inherits needed? Otherwise it doesn't take the font-size and color
from the surrounding light dom?

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
Hmm... why is this necessary? I thought not-selected animated pages were
automatically not visible?

Is this because of the above CSS rule applying to neon-animated-pages > div?

[Moe, 2016-05-04 17:33:52]

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/import_supervised_user.html
(right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/import_supervised_user.html:66:
font-size: inherit;
On 2016/05/04 17:04:11, tommycli wrote:
> Are these inherits needed? Otherwise it doesn't take the font-size and color
> from the surrounding light dom?

They are. paper-item applies 'paper-font-subhead' mixin which defines font-size
and line-height. I'd like those to be inherited from paper-listbox which in turn
(implicitly) inherits them from its parent and so on...
for the color of a disabled paper-item, I'd like it not to change. So I'm
inheriting it from paper-listbox which (explicitly) inherits it from its parent
ans so on.

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
On 2016/05/04 17:04:11, tommycli wrote:
> Hmm... why is this necessary? I thought not-selected animated pages were
> automatically not visible?
> 
> Is this because of the above CSS rule applying to neon-animated-pages > div?

neon-animated-pages works as expected. Note that if you look at the full source
of this file, user-manager-pages has its own content element (other than the
extra one that i removed). Q: why does it have its own content element? b/c for
countless reasons i can't componentize its distributed children and bring them
into this file! One reason being that the user pods JS/logic is complex and
shared between cros and desktop.

neon-animated-pages hides non-selected pages which are the wrapper divs (<div
id="...">). That does not penetrate to distributed children of
user-manager-pages. this new rules basically says "if a wrapper div is not
selected and has a <content> tag as a child, go ahead and hide the elements
under <content> as well."

'!important' is also needed b/c some of those children set 'display: block'
using id which is more specific and therefore takes priority.

[tommycli, 2016-05-04 17:56:02]

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
On 2016/05/04 17:33:52, Moe wrote:
> On 2016/05/04 17:04:11, tommycli wrote:
> > Hmm... why is this necessary? I thought not-selected animated pages were
> > automatically not visible?
> > 
> > Is this because of the above CSS rule applying to neon-animated-pages > div?
> 
> neon-animated-pages works as expected. Note that if you look at the full
source
> of this file, user-manager-pages has its own content element (other than the
> extra one that i removed). Q: why does it have its own content element? b/c
for
> countless reasons i can't componentize its distributed children and bring them
> into this file! One reason being that the user pods JS/logic is complex and
> shared between cros and desktop.
> 
> neon-animated-pages hides non-selected pages which are the wrapper divs (<div
> id="...">). That does not penetrate to distributed children of
> user-manager-pages. this new rules basically says "if a wrapper div is not
> selected and has a <content> tag as a child, go ahead and hide the elements
> under <content> as well."
> 
> '!important' is also needed b/c some of those children set 'display: block'
> using id which is more specific and therefore takes priority.

Hmm... I see. Do you still need this rule if the isPageVisible_ dom-if
statements are gone?

I suspect (though I'm not sure) that the <content></content> tag is not stamped
until after isPageVisible_ is true, and that's why the light-dom content is not
auto-hidden.

[tommycli, 2016-05-04 18:58:12]

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
On 2016/05/04 17:56:02, tommycli wrote:
> On 2016/05/04 17:33:52, Moe wrote:
> > On 2016/05/04 17:04:11, tommycli wrote:
> > > Hmm... why is this necessary? I thought not-selected animated pages were
> > > automatically not visible?
> > > 
> > > Is this because of the above CSS rule applying to neon-animated-pages >
div?
> > 
> > neon-animated-pages works as expected. Note that if you look at the full
> source
> > of this file, user-manager-pages has its own content element (other than the
> > extra one that i removed). Q: why does it have its own content element? b/c
> for
> > countless reasons i can't componentize its distributed children and bring
them
> > into this file! One reason being that the user pods JS/logic is complex and
> > shared between cros and desktop.
> > 
> > neon-animated-pages hides non-selected pages which are the wrapper divs
(<div
> > id="...">). That does not penetrate to distributed children of
> > user-manager-pages. this new rules basically says "if a wrapper div is not
> > selected and has a <content> tag as a child, go ahead and hide the elements
> > under <content> as well."
> > 
> > '!important' is also needed b/c some of those children set 'display: block'
> > using id which is more specific and therefore takes priority.
> 
> Hmm... I see. Do you still need this rule if the isPageVisible_ dom-if
> statements are gone?
> 
> I suspect (though I'm not sure) that the <content></content> tag is not
stamped
> until after isPageVisible_ is true, and that's why the light-dom content is
not
> auto-hidden.

I just spoke to michaelpg about this topic. Basically - <content> should never
be within a dom-if. Bad things happen.

[Moe, 2016-05-04 19:37:40]

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
On 2016/05/04 18:58:12, tommycli wrote:
> On 2016/05/04 17:56:02, tommycli wrote:
> > On 2016/05/04 17:33:52, Moe wrote:
> > > On 2016/05/04 17:04:11, tommycli wrote:
> > > > Hmm... why is this necessary? I thought not-selected animated pages were
> > > > automatically not visible?
> > > > 
> > > > Is this because of the above CSS rule applying to neon-animated-pages >
> div?
> > > 
> > > neon-animated-pages works as expected. Note that if you look at the full
> > source
> > > of this file, user-manager-pages has its own content element (other than
the
> > > extra one that i removed). Q: why does it have its own content element?
b/c
> > for
> > > countless reasons i can't componentize its distributed children and bring
> them
> > > into this file! One reason being that the user pods JS/logic is complex
and
> > > shared between cros and desktop.
> > > 
> > > neon-animated-pages hides non-selected pages which are the wrapper divs
> (<div
> > > id="...">). That does not penetrate to distributed children of
> > > user-manager-pages. this new rules basically says "if a wrapper div is not
> > > selected and has a <content> tag as a child, go ahead and hide the
elements
> > > under <content> as well."
> > > 
> > > '!important' is also needed b/c some of those children set 'display:
block'
> > > using id which is more specific and therefore takes priority.
> > 
> > Hmm... I see. Do you still need this rule if the isPageVisible_ dom-if
> > statements are gone?
> > 
> > I suspect (though I'm not sure) that the <content></content> tag is not
> stamped
> > until after isPageVisible_ is true, and that's why the light-dom content is
> not
> > auto-hidden.
> 
> I just spoke to michaelpg about this topic. Basically - <content> should never
> be within a dom-if. Bad things happen.

I took content out of dom-if. user-pods-page is the default page anyway. so let
it get stamped right away. However, we still seem to need this rule. setting
'display: none' on <content> doesn't seem to propagate to its children. (based
on chrome's accessibility extension test results).

[tommycli, 2016-05-04 20:16:57]

lgtm sans below: thanks

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/user_manager_pages.html (right):

https://codereview.chromium.org/1944463002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/user_manager_pages.html:21:
neon-animated-pages > div:not(.iron-selected) > ::content > * {
On 2016/05/04 19:37:40, Moe wrote:
> On 2016/05/04 18:58:12, tommycli wrote:
> > On 2016/05/04 17:56:02, tommycli wrote:
> > > On 2016/05/04 17:33:52, Moe wrote:
> > > > On 2016/05/04 17:04:11, tommycli wrote:
> > > > > Hmm... why is this necessary? I thought not-selected animated pages
were
> > > > > automatically not visible?
> > > > > 
> > > > > Is this because of the above CSS rule applying to neon-animated-pages
>
> > div?
> > > > 
> > > > neon-animated-pages works as expected. Note that if you look at the full
> > > source
> > > > of this file, user-manager-pages has its own content element (other than
> the
> > > > extra one that i removed). Q: why does it have its own content element?
> b/c
> > > for
> > > > countless reasons i can't componentize its distributed children and
bring
> > them
> > > > into this file! One reason being that the user pods JS/logic is complex
> and
> > > > shared between cros and desktop.
> > > > 
> > > > neon-animated-pages hides non-selected pages which are the wrapper divs
> > (<div
> > > > id="...">). That does not penetrate to distributed children of
> > > > user-manager-pages. this new rules basically says "if a wrapper div is
not
> > > > selected and has a <content> tag as a child, go ahead and hide the
> elements
> > > > under <content> as well."
> > > > 
> > > > '!important' is also needed b/c some of those children set 'display:
> block'
> > > > using id which is more specific and therefore takes priority.
> > > 
> > > Hmm... I see. Do you still need this rule if the isPageVisible_ dom-if
> > > statements are gone?
> > > 
> > > I suspect (though I'm not sure) that the <content></content> tag is not
> > stamped
> > > until after isPageVisible_ is true, and that's why the light-dom content
is
> > not
> > > auto-hidden.
> > 
> > I just spoke to michaelpg about this topic. Basically - <content> should
never
> > be within a dom-if. Bad things happen.
> 
> I took content out of dom-if. user-pods-page is the default page anyway. so
let
> it get stamped right away. However, we still seem to need this rule. setting
> 'display: none' on <content> doesn't seem to propagate to its children. (based
> on chrome's accessibility extension test results).

Okay that's fine, can you just add a comment above explaining that
neon-animated-pages doesn't hide the content automatically?

[Moe, 2016-05-04 21:54:03]

mahmadi@chromium.org changed reviewers:
+ michaelpg@chromium.org

[Moe, 2016-05-04 21:54:03]

Thank you tommy!

Michael, could you please review this CL?

[michaelpg, 2016-05-05 20:09:23]

lgtm

https://codereview.chromium.org/1944463002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/import_supervised_user.html
(right):

https://codereview.chromium.org/1944463002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/import_supervised_user.html:111: <span
id="title">[$i18n{supervisedUserImportTitle}</span>
remove [

[Moe, 2016-05-06 14:00:58]

Thank you!

https://codereview.chromium.org/1944463002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_user_manager/import_supervised_user.html
(right):

https://codereview.chromium.org/1944463002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_user_manager/import_supervised_user.html:111: <span
id="title">[$i18n{supervisedUserImportTitle}</span>
On 2016/05/05 20:09:22, michaelpg wrote:
> remove [

Done.

[Moe, 2016-05-06 14:01:03]

The CQ bit was checked by mahmadi@chromium.org

[Moe, 2016-05-06 14:01:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1944463002/#ps80001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-05-06 14:01:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1944463002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1944463002/80001

[commit-bot: I haz the power, 2016-05-06 14:20:17]

Description was changed from

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-05-06 14:20:19]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-06 14:21:41]

Description was changed from

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD User Manager: fixes accessibility issues

1. Uses system's default focus behavior profile avatar icons.
2. Uses paper-menu in import-supervised-user dialog for accessibility (and
updates browser tests accordingly).
3. Hides the user pods page properly when the page is not visible so its
elements won't get focus.
4. Also removes redundant 'content' tag from user-manager-page.

BUG=563722
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/1aa2dbfa94ccc24698fe5ffd13573a5f68cea0d3
Cr-Commit-Position: refs/heads/master@{#392053}
==========

[commit-bot: I haz the power, 2016-05-06 14:21:42]

Patchset 5 (id:??) landed as
https://crrev.com/1aa2dbfa94ccc24698fe5ffd13573a5f68cea0d3
Cr-Commit-Position: refs/heads/master@{#392053}