Skip to content

Commit 65f53dc

Browse files
drgrice1drgrice1
committed
Remove aria labels from radio buttons.
This was requested by @glarose and the accessibility experts at the University of Michigan. Having aria labels on these radio buttons that also have labels causes issues with screen readers. It results in the aria label being read and not the actual label. It seems to me that this is correct in any case. An input should not have both a label and an aria label. Those clearly conflict. Also improve the aria labels for the answers inside the radio buttons for a `RadioMultiAnswer`. For this the `parserRadioMultiAnswer.pl` macro has its own generate_aria_label method for now as the `generate_aria_label` method in `PGbasicmacros.pl` does not work well for the `parserRadioMultiAnswer.pl` macro. In addition add some missing `maketext` calls on aria labels, and take advantage that `maketext` substitutions now work in PG. We need to find a better way to create aria labels in general. The `generate_aria_label` method in `PGbasicmacros.pl` is neither versatile nor extensible. Attempting to construct meaningful labels from PG's answer name was never a good idea to begin with. By the way, since the aria label is currently constructed from the answer name, this is yet another reason to strongly discourage (actually flat out forbid) problem authors from using hard coded made up answer names in problems instead of calling `NEW_ANS_NAME` to obtain one.
1 parent fa82e38 commit 65f53dc

File tree

6 files changed

+63
-30
lines changed

6 files changed

+63
-30
lines changed

‎lib/Value/AnswerChecker.pm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ sub ans_matrix {
459459
foreach my $j (0 .. $cols - 1) {
460460
my $label;
461461
if ($options{aria_label}) {
462-
$label = $options{aria_label} . 'row ' . ($i + 1) . ' col ' . ($j + 1);
462+
$label = $options{aria_label} . main::maketext('row [_1] col [_2] ', $i + 1, $j + 1);
463463
} else {
464464
$label = pgCall('generate_aria_label', ANS_NAME($ename, $i, $j));
465465
}

‎macros/core/PGbasicmacros.pl

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -467,11 +467,10 @@ sub NAMED_ANS_RADIO {
467467
'label',
468468
tag(
469469
'input',
470-
type => 'radio',
471-
name => $name,
472-
id => $name,
473-
aria_label => $options{aria_label} // generate_aria_label($name) . 'option 1 ',
474-
value => $value,
470+
type => 'radio',
471+
name => $name,
472+
id => $name,
473+
value => $value,
475474
$checked ? (checked => undef) : (),
476475
%{ $options{attributes} }
477476
)
@@ -499,11 +498,10 @@ sub NAMED_ANS_RADIO_EXTENSION {
499498
'label',
500499
tag(
501500
'input',
502-
type => 'radio',
503-
name => $name,
504-
id => $options{id} // "${name}_$value",
505-
aria_label => $options{aria_label} // generate_aria_label($name),
506-
value => $value,
501+
type => 'radio',
502+
name => $name,
503+
id => $options{id} // "${name}_$value",
504+
value => $value,
507505
$checked ? (checked => undef) : (),
508506
%{ $options{attributes} }
509507
)
@@ -541,30 +539,30 @@ sub generate_aria_label {
541539

542540
# if we dont have an AnSwEr type name then we do the best we can
543541
if ($name !~ /AnSwEr\d+/) {
544-
return maketext('answer') . ' ' . $name;
542+
return maketext('answer [_1] ', $name);
545543
}
546544

547545
# check for quiz prefix
548546
if ($name =~ /^Q\d+/ || $name =~ /^MaTrIx_Q\d+/) {
549547
$name =~ s/Q0*(\d+)_//;
550-
$label .= maketext('problem') . ' ' . $1 . ' ';
548+
$label .= maketext('problem [_1] ', $1);
551549
}
552550

553551
# get answer number
554552
$name =~ /AnSwEr0*(\d+)/;
555-
$label .= maketext('answer') . ' ' . $1 . ' ';
553+
$label .= maketext('answer [_1] ', $1);
556554

557555
# check for Multianswer
558556
if ($name =~ /MuLtIaNsWeR_/) {
559557
$name =~ s/MuLtIaNsWeR_//;
560558
$name =~ /AnSwEr(\d+)_(\d+)/;
561-
$label .= maketext('part') . ' ' . ($2 + 1) . ' ';
559+
$label .= maketext('part [_1] ', $2 + 1);
562560
}
563561

564562
# check for Matrix
565563
if ($name =~ /^MaTrIx_/) {
566564
$name =~ /_(\d+)_(\d+)$/;
567-
$label .= maketext('row') . ' ' . ($1 + 1) . ' ' . maketext('column') . ' ' . ($2 + 1) . ' ';
565+
$label .= maketext('row [_1] column [_2] ', $1 + 1, $2 + 1);
568566
}
569567

570568
return $label;
@@ -624,7 +622,7 @@ sub NAMED_ANS_CHECKBOX {
624622
type => 'checkbox',
625623
name => $name,
626624
id => $name,
627-
aria_label => generate_aria_label($name) . 'option 1 ',
625+
aria_label => $options{aria_label} // (generate_aria_label($name) . maketext('option [_1] ', 1)),
628626
value => $value,
629627
$checked ? (checked => undef) : (),
630628
%{ $options{attributes} }
@@ -677,7 +675,9 @@ sub NAMED_ANS_CHECKBOX_BUTTONS {
677675
while (@buttons) {
678676
$value = shift @buttons;
679677
$tag = shift @buttons;
680-
push(@out, NAMED_ANS_CHECKBOX_OPTION($name, $value, $tag, aria_label => $label . "option $count "));
678+
push(@out,
679+
NAMED_ANS_CHECKBOX_OPTION($name, $value, $tag, aria_label => $label . maketext('option [_1] ', $count))
680+
);
681681
$count++;
682682
}
683683

‎macros/parsers/parserCheckboxList.pl

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ sub CHECKS {
553553

554554
my @checks;
555555
main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name;
556-
my $label = main::generate_aria_label($name);
556+
my $label = (delete $options{aria_label}) // main::generate_aria_label($name);
557557

558558
for my $i (0 .. $#{ $self->{orderedChoices} }) {
559559
my $value = $self->{values}[$i];
@@ -567,12 +567,18 @@ sub CHECKS {
567567
main::NAMED_ANS_CHECKBOX_OPTION(
568568
$name, $value, " $tag",
569569
id => "${name}_$i",
570-
aria_label => $label . 'option ' . ($i + 1) . ' ',
570+
aria_label => $label . main::maketext('option [_1] ', $i + 1),
571571
%options
572572
)
573573
);
574574
} else {
575-
push(@checks, main::NAMED_ANS_CHECKBOX($name, $value, " $tag", $extend, %options));
575+
push(
576+
@checks,
577+
main::NAMED_ANS_CHECKBOX(
578+
$name, $value, " $tag", $extend, %options,
579+
aria_label => $label . main::maketext('option [_1] ', 1)
580+
)
581+
);
576582
}
577583
}
578584

‎macros/parsers/parserPopUp.pl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ sub MENU {
419419
my $menu = "";
420420
main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name;
421421
my $answer_value = (defined($main::inputs_ref->{$name}) ? $main::inputs_ref->{$name} : '');
422-
my $aria_label = main::generate_aria_label($name);
422+
my $aria_label = $options{aria_label} // main::generate_aria_label($name);
423423

424424
if ($main::displayMode =~ m/^HTML/) {
425425
if ($self->{useHTMLSelect}) {

‎macros/parsers/parserRadioButtons.pl

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,6 @@ sub BUTTONS {
627627
my @choices = @{ $self->{orderedChoices} };
628628
my @radio = ();
629629
main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name;
630-
my $label = main::generate_aria_label($name);
631630

632631
foreach my $i (0 .. $#choices) {
633632
my $value = $self->{values}[$i];
@@ -639,8 +638,7 @@ sub BUTTONS {
639638
@radio,
640639
main::NAMED_ANS_RADIO_EXTENSION(
641640
$name, $value, $tag,
642-
aria_label => $label . "option " . ($i + 1) . " ",
643-
id => "${name}_$i",
641+
id => "${name}_$i",
644642
$self->{uncheckable}
645643
? (
646644
attributes => {

‎macros/parsers/parserRadioMultiAnswer.pl

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,34 @@ sub label {
620620
return $self->{labels}[$i];
621621
}
622622

623+
sub generate_aria_label {
624+
my ($name, $radioIndex, $partIndex) = @_;
625+
my $label = '';
626+
627+
$name =~ s/$answerPrefix//;
628+
629+
# Check for the quiz prefix.
630+
if ($name =~ /^Q\d+/ || $name =~ /^MaTrIx_Q\d+/) {
631+
$name =~ s/Q0*(\d+)_//;
632+
$label .= main::maketext('problem [_1] ', $1);
633+
}
634+
635+
# Get the answer number.
636+
$name =~ /AnSwEr0*(\d+)/;
637+
$label .= main::maketext('answer [_1] ', $1);
638+
639+
$label .= main::maketext('part [_1] ', $radioIndex);
640+
$label .= main::maketext('subpart [_1] ', $partIndex);
641+
642+
# Check for a Matrix answer.
643+
if ($name =~ /^MaTrIx_/) {
644+
$name =~ /_(\d+)_(\d+)$/;
645+
$label .= main::maketext('row [_1] column [_2] ', $1 + 1, $2 + 1);
646+
}
647+
648+
return $label;
649+
}
650+
623651
# Produce the answer rule.
624652
sub ans_rule {
625653
my ($self, $size, @options) = @_;
@@ -648,6 +676,7 @@ sub ans_rule {
648676
? (defined $size->[$i][ $_ - 1 ] ? $size->[$i][ $_ - 1 ] : 20)
649677
: $size,
650678
answer_group_name => $radio_name,
679+
aria_label => generate_aria_label($name, $i + 1, $_),
651680
@options
652681
)
653682
)
@@ -663,6 +692,7 @@ sub ans_rule {
663692
? (defined $size->[$i][ $_ - 1 ] ? $size->[$i][ $_ - 1 ] : 20)
664693
: $size,
665694
answer_group_name => $radio_name,
695+
aria_label => generate_aria_label($name, $i + 1, $_),
666696
@options
667697
)
668698
)
@@ -724,11 +754,10 @@ sub begin_radio {
724754
HTML => qq{<div class="radio-container">}
725755
. main::tag(
726756
'input',
727-
type => 'radio',
728-
name => $name,
729-
id => "$name$idSuffix",
730-
aria_label => main::generate_aria_label("$answerPrefix${name}_0") . ' option ' . ($i + 1),
731-
value => $value,
757+
type => 'radio',
758+
name => $name,
759+
id => "$name$idSuffix",
760+
value => $value,
732761
$self->{uncheckable}
733762
? (
734763
data_uncheckable_radio => 1,

0 commit comments

Comments
 (0)