search

phetsims / inverse-square-law-common

GNU General Public License v3.0
1 stars 2 forks source link

ISLC Checkbox appendLabel default? #74

Closed zepumph closed 5 years ago

zepumph commented 5 years ago

ISLC uses appendLabel: false, but Checkbox.js by default appends the label.

@terracoda is there a reason why ISLC checkboxes have the label before the checkbox, but all other checkboxes have the label after?

My preference, if there is no strong reason otherwise, would be to remove the override in ISLC and have all checkboxes behave the same. This would mean the ISLC checkboxes would change so that the checkbox would be before the label.

What do you think?

terracoda commented 5 years ago

@zepumph, I think in the PDOM labels have always come before the actual checkbox element.

Can you verify that some how?

terracoda commented 5 years ago

Hmm, @zepumph, for regular visual checkboxes, it might be ok to have the input first. I'm looking into this.

terracoda commented 5 years ago

@zepumph, looks like I have been inconsistent in terms of where we put the labels in the PDOM.

I just did a review of some our latest A11y Sims. And this is what I found:

  1. BASE and MAL radiobutton labels come after the radio inputs
  2. BASE’s checkbox is special - uses an aria-label attribute
  3. RIAW (and OL’s), and MAL, GFLB, GFL all the slider labels come first
  4. Molarity Solution Values checkbox - the label comes after the checkbox input. AND I noticed that the name, "Solution Values", is spoken twice automatically when the checkbox input receives focus!!!!
  5. GFL and GFLB checkboxes - the labels come before the checkboxes, and the name IS NOT spoken twice upon focus of the checkbox input.

@zepumph, I was about to say go ahead and switch things around, but then I found the label duplication in Molarity. Perhaps, there was a reason the labels come before the inputs in the PDOM?

Can you verify that the order is the only difference between Molarity's and GLFB's checkboxes?

zepumph commented 5 years ago

@terracoda I could not reproduce the duplication in Chrome/windows. I also couldn't see any differences between the GFL and Molarity checkboxes. Above I removed the ISLC specific override for the checkbox label position. So now Molarity and and GFLB and GFL should all have the label after the checkbox. Does this make the ISLC sims have a repeated label like we were seeing in Molarity?

This change was meant as a test to understand the problem you are seeing more clearly. But it is also the ideal for the code in ISLC. I don't really want ISLC checkboxes behaving differently from others, since there isn't really any reason to.

terracoda commented 5 years ago

@zepumph, yes, I get duplication on all checkboxes now (Molarity, GFL, GFLB). I thought I was going a little crazy in https://github.com/phetsims/molarity/issues/133#issue since I hadn't noticed this issue before commenting and closing https://github.com/phetsims/molarity/issues/133#issuecomment-518321952 ...

I have no idea why I was not hearing duplication in GFL and GFLB before your change in ISLC.

I had a good long discussion with Matt King (ARIA and screen reader expert). According to Matt, for screen reader users there is no functional difference if the label comes before or after the input. The only current solution to duplicate checkbox labels is to nest the inputs inside the labels eliminating the need for the HTML for attribute. This nesting option is not available to us in Scenery.

Design wise there was one reason I liked having the labels first, the reading order seemed more logical to me:

But really, duplication is a bigger issue for me than order. If having the labels first removes duplication, I think this should be the pattern we adopt for all checkboxes.

zepumph commented 5 years ago

Oh sorry, I'm just seeing this now. @terracoda please assign me to any issue you would like my feedback on.

But really, duplication is a bigger issue for me than order. If having the labels first removes duplication, I think this should be the pattern we adopt for all checkboxes.

That sounds good. I will make the label first on all checkboxes.

zepumph commented 5 years ago

@terracoda please review. All checkboxes in the project should now prepend the label, so order should be: label, input, help text.

terracoda commented 5 years ago

@zepumph, I verified that I am no longer hearing duplication in any of the checkboxes (GFLB, GFL, Molarity).

I have no idea why having the labels after the checkboxes causes duplication, but I think this is something I should document somewhere, maybe in the design pattern for checkbox.

Thanks for making the change. Closing issue.

terracoda commented 5 years ago

@zepumph I added a note to https://github.com/phetsims/sun/blob/master/doc/Checkbox.md