MD-Settings A11y: Disable radio list if all options are disabled.

MD-Settings A11y: Disable radio list if all options are disabled.

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

Review-Url: https://codereview.chromium.org/2715763002
Cr-Commit-Position: refs/heads/master@{#461915}
Committed: https://chromium.googlesource.com/chromium/src/+/4d9d2432508df3c21fec9178132362c52023dae2

[hcarmona, 2017-02-23 18:28:49]

Description was changed from

==========
MD-Settings A11y: No tabindex on radio list if options are all disabled.

BUG=642644
==========

to

==========
MD-Settings A11y: No tabindex on radio list if options are all disabled.

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

[hcarmona, 2017-02-23 18:28:51]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[hcarmona, 2017-02-23 18:29:22]

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

[hcarmona, 2017-02-23 18:29:22]

PTAL

[commit-bot: I haz the power, 2017-02-23 18:29:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2017-02-23 19:01:59]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2017-02-23 19:01:59]

changing to tabindex="-1" disables tab but still allows clicking on something to
focus it.

just remove the tabindex attribute altogether to remove it from being
"focusable" (focused on click) as well as not be in the "sequential tab order"
(pressing Tab)

[hcarmona, 2017-02-23 19:09:05]

On 2017/02/23 19:01:59, Dan Beam wrote:
> changing to tabindex="-1" disables tab but still allows clicking on something
to
> focus it.
> 
> just remove the tabindex attribute altogether to remove it from being
> "focusable" (focused on click) as well as not be in the "sequential tab order"
> (pressing Tab)

The paper-radio-group adds its own tabindex, so not adding one results in the
default tabindex of 0.
https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chro...

What can we do in this case?

[Dan Beam, 2017-02-23 19:13:36]

On 2017/02/23 19:09:05, hcarmona wrote:
> On 2017/02/23 19:01:59, Dan Beam wrote:
> > changing to tabindex="-1" disables tab but still allows clicking on
something
> to
> > focus it.
> > 
> > just remove the tabindex attribute altogether to remove it from being
> > "focusable" (focused on click) as well as not be in the "sequential tab
order"
> > (pressing Tab)
> 
> The paper-radio-group adds its own tabindex, so not adding one results in the
> default tabindex of 0.
>
https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chro...
> 
> What can we do in this case?

i knew it used hostAttributes for a default of 0.  figure out a way to undo this
;)

[commit-bot: I haz the power, 2017-02-23 19:45:00]

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

[commit-bot: I haz the power, 2017-02-23 19:45:01]

Dry run: This issue passed the CQ dry run.

[hcarmona, 2017-02-23 20:07:47]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 20:08:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2017-02-23 20:09:28]

On 2017/02/23 19:13:36, Dan Beam wrote:
> On 2017/02/23 19:09:05, hcarmona wrote:
> > On 2017/02/23 19:01:59, Dan Beam wrote:
> > > changing to tabindex="-1" disables tab but still allows clicking on
> something
> > to
> > > focus it.
> > > 
> > > just remove the tabindex attribute altogether to remove it from being
> > > "focusable" (focused on click) as well as not be in the "sequential tab
> order"
> > > (pressing Tab)
> > 
> > The paper-radio-group adds its own tabindex, so not adding one results in
the
> > default tabindex of 0.
> >
>
https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chro...
> > 
> > What can we do in this case?
> 
> i knew it used hostAttributes for a default of 0.  figure out a way to undo
this
> ;)

PTAL, if this looks good I'll create a PR that updates the paper-radio-group

[Dan Beam, 2017-02-23 20:49:15]

yeah, i think this'll work!

https://codereview.chromium.org/2715763002/diff/20001/third_party/polymer/v1_...
File
third_party/polymer/v1_0/components-chromium/paper-radio-group/paper-radio-group-extracted.js
(right):

https://codereview.chromium.org/2715763002/diff/20001/third_party/polymer/v1_...
third_party/polymer/v1_0/components-chromium/paper-radio-group/paper-radio-group-extracted.js:53:
observer: '_onDisableChange',
nit: changed

[Dan Beam, 2017-02-23 20:51:45]

also, fwiw, i just looked back through all the behaviors this element pulls in
(directly or indirectly) and they don't seem to have a disable that this would
override (but they do have checks for .hasAttribute('disabled'), so we might
also consider reflectToAttribute?)

[commit-bot: I haz the power, 2017-02-23 21:22:54]

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

[commit-bot: I haz the power, 2017-02-23 21:22:55]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[hcarmona, 2017-02-23 23:15:12]

Patchset #3 (id:40001) has been deleted

[hcarmona, 2017-02-23 23:16:29]

Thanks for the feedback! I've created a PR here:
https://github.com/PolymerElements/paper-radio-group/pull/77

[Dan Beam, 2017-03-10 08:00:06]

what's the status of this or polymer PRs?

[Dan Beam, 2017-03-17 04:10:31]

do this locally in third_party/polymer if polymer reviews take too long, imo

[hcarmona, 2017-03-17 18:57:15]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 18:57:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2017-03-17 19:04:32]

Updated w/ the changes we expect in the PRs

For quick reference, the PRs:
https://github.com/PolymerElements/iron-menu-behavior/pull/88
https://github.com/PolymerElements/paper-radio-group/pull/77

I didn't bring the polymer tests. If we run these in chromium, then I can add
them to  this CL.

I had some presubmit warnings which I felt were okay to bypass... let me know if
I need to fix something:
  - 'README.chromium' file should also be updated with correct version and
package info
  - Found components that are not listed in bower.json
      app-layout

[commit-bot: I haz the power, 2017-03-17 19:10:15]

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

[commit-bot: I haz the power, 2017-03-17 19:10:16]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Dan Beam, 2017-03-17 23:58:53]

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
File third_party/polymer/v1_0/chromium.patch (right):

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
third_party/polymer/v1_0/chromium.patch:29: ---
a/third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js
this is the wrong path.  please check that your patch works with this line from
inside the third_party/polymer/v1_0/ directory

https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/reproduce.sh?q=...

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
File
third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js
(right):

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js:242:
this._disableTabindex(!couldTab);
indent off

[hcarmona, 2017-03-18 00:40:07]

Description was changed from

==========
MD-Settings A11y: No tabindex on radio list if options are all disabled.

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

to

==========
MD-Settings A11y: No tabindex on radio list if options are all disabled.

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

[hcarmona, 2017-03-18 01:04:12]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[hcarmona, 2017-03-18 01:05:07]

Applied feedback. (also, issue was closed for some reason, reopened)

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
File third_party/polymer/v1_0/chromium.patch (right):

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
third_party/polymer/v1_0/chromium.patch:29: ---
a/third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js
On 2017/03/17 23:58:53, Dan Beam wrote:
> this is the wrong path.  please check that your patch works with this line
from
> inside the third_party/polymer/v1_0/ directory
> 
>
https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/reproduce.sh?q=...

Done.

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
File
third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js
(right):

https://codereview.chromium.org/2715763002/diff/100001/third_party/polymer/v1...
third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js:242:
this._disableTabindex(!couldTab);
On 2017/03/17 23:58:53, Dan Beam wrote:
> indent off

Done.

[commit-bot: I haz the power, 2017-03-18 01:05:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-18 02:32:49]

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

[commit-bot: I haz the power, 2017-03-18 02:32:50]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-03-21 07:13:23]

bicknellr@ is responding in the PR, so let's keep going with that for now

[hcarmona, 2017-04-04 19:29:31]

Patchset #7 (id:140001) has been deleted

[hcarmona, 2017-04-04 20:34:45]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 20:36:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2017-04-04 20:38:09]

Updated to depend on pulling polymer changes.

[hcarmona, 2017-04-04 20:51:54]

Description was changed from

==========
MD-Settings A11y: No tabindex on radio list if options are all disabled.

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

to

==========
MD-Settings A11y: Disable radio list if all options are disabled.

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

[commit-bot: I haz the power, 2017-04-04 21:59:48]

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

[commit-bot: I haz the power, 2017-04-04 21:59:49]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-04-04 22:02:27]

lgtm

[hcarmona, 2017-04-05 00:20:26]

The CQ bit was checked by hcarmona@chromium.org

[commit-bot: I haz the power, 2017-04-05 00:21:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 00:30:42]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1491351626329700,
"parent_rev": "c1d1aee5dcc8f13668fc418d5d670dc280759531", "commit_rev":
"4d9d2432508df3c21fec9178132362c52023dae2"}

[commit-bot: I haz the power, 2017-04-05 00:39:03]

Description was changed from

==========
MD-Settings A11y: Disable radio list if all options are disabled.

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

to

==========
MD-Settings A11y: Disable radio list if all options are disabled.

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

Review-Url: https://codereview.chromium.org/2715763002
Cr-Commit-Position: refs/heads/master@{#461915}
Committed:
https://chromium.googlesource.com/chromium/src/+/4d9d2432508df3c21fec91781323...
==========

[commit-bot: I haz the power, 2017-04-05 00:39:04]

Committed patchset #7 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/4d9d2432508df3c21fec91781323...