From c7b44e7374de319a16368d101c9bf3ba9bb54ead Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 3 Apr 2025 20:45:11 -0500 Subject: [PATCH] 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. --- lib/Value/AnswerChecker.pm | 2 +- macros/core/PGbasicmacros.pl | 34 ++++++++++----------- macros/parsers/parserCheckboxList.pl | 12 ++++++-- macros/parsers/parserPopUp.pl | 2 +- macros/parsers/parserRadioButtons.pl | 4 +-- macros/parsers/parserRadioMultiAnswer.pl | 39 +++++++++++++++++++++--- 6 files changed, 63 insertions(+), 30 deletions(-) diff --git a/lib/Value/AnswerChecker.pm b/lib/Value/AnswerChecker.pm index 16210cb9f..f5b0b463a 100644 --- a/lib/Value/AnswerChecker.pm +++ b/lib/Value/AnswerChecker.pm @@ -459,7 +459,7 @@ sub ans_matrix { foreach my $j (0 .. $cols - 1) { my $label; if ($options{aria_label}) { - $label = $options{aria_label} . 'row ' . ($i + 1) . ' col ' . ($j + 1); + $label = $options{aria_label} . main::maketext('row [_1] col [_2] ', $i + 1, $j + 1); } else { $label = pgCall('generate_aria_label', ANS_NAME($ename, $i, $j)); } diff --git a/macros/core/PGbasicmacros.pl b/macros/core/PGbasicmacros.pl index 2cd93e486..0eddb168b 100644 --- a/macros/core/PGbasicmacros.pl +++ b/macros/core/PGbasicmacros.pl @@ -467,11 +467,10 @@ sub NAMED_ANS_RADIO { 'label', tag( 'input', - type => 'radio', - name => $name, - id => $name, - aria_label => $options{aria_label} // generate_aria_label($name) . 'option 1 ', - value => $value, + type => 'radio', + name => $name, + id => $name, + value => $value, $checked ? (checked => undef) : (), %{ $options{attributes} } ) @@ -499,11 +498,10 @@ sub NAMED_ANS_RADIO_EXTENSION { 'label', tag( 'input', - type => 'radio', - name => $name, - id => $options{id} // "${name}_$value", - aria_label => $options{aria_label} // generate_aria_label($name), - value => $value, + type => 'radio', + name => $name, + id => $options{id} // "${name}_$value", + value => $value, $checked ? (checked => undef) : (), %{ $options{attributes} } ) @@ -541,30 +539,30 @@ sub generate_aria_label { # if we dont have an AnSwEr type name then we do the best we can if ($name !~ /AnSwEr\d+/) { - return maketext('answer') . ' ' . $name; + return maketext('answer [_1] ', $name); } # check for quiz prefix if ($name =~ /^Q\d+/ || $name =~ /^MaTrIx_Q\d+/) { $name =~ s/Q0*(\d+)_//; - $label .= maketext('problem') . ' ' . $1 . ' '; + $label .= maketext('problem [_1] ', $1); } # get answer number $name =~ /AnSwEr0*(\d+)/; - $label .= maketext('answer') . ' ' . $1 . ' '; + $label .= maketext('answer [_1] ', $1); # check for Multianswer if ($name =~ /MuLtIaNsWeR_/) { $name =~ s/MuLtIaNsWeR_//; $name =~ /AnSwEr(\d+)_(\d+)/; - $label .= maketext('part') . ' ' . ($2 + 1) . ' '; + $label .= maketext('part [_1] ', $2 + 1); } # check for Matrix if ($name =~ /^MaTrIx_/) { $name =~ /_(\d+)_(\d+)$/; - $label .= maketext('row') . ' ' . ($1 + 1) . ' ' . maketext('column') . ' ' . ($2 + 1) . ' '; + $label .= maketext('row [_1] column [_2] ', $1 + 1, $2 + 1); } return $label; @@ -624,7 +622,7 @@ sub NAMED_ANS_CHECKBOX { type => 'checkbox', name => $name, id => $name, - aria_label => generate_aria_label($name) . 'option 1 ', + aria_label => $options{aria_label} // (generate_aria_label($name) . maketext('option [_1] ', 1)), value => $value, $checked ? (checked => undef) : (), %{ $options{attributes} } @@ -677,7 +675,9 @@ sub NAMED_ANS_CHECKBOX_BUTTONS { while (@buttons) { $value = shift @buttons; $tag = shift @buttons; - push(@out, NAMED_ANS_CHECKBOX_OPTION($name, $value, $tag, aria_label => $label . "option $count ")); + push(@out, + NAMED_ANS_CHECKBOX_OPTION($name, $value, $tag, aria_label => $label . maketext('option [_1] ', $count)) + ); $count++; } diff --git a/macros/parsers/parserCheckboxList.pl b/macros/parsers/parserCheckboxList.pl index 9b87e7974..f0da9076b 100644 --- a/macros/parsers/parserCheckboxList.pl +++ b/macros/parsers/parserCheckboxList.pl @@ -553,7 +553,7 @@ sub CHECKS { my @checks; main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name; - my $label = main::generate_aria_label($name); + my $label = (delete $options{aria_label}) // main::generate_aria_label($name); for my $i (0 .. $#{ $self->{orderedChoices} }) { my $value = $self->{values}[$i]; @@ -567,12 +567,18 @@ sub CHECKS { main::NAMED_ANS_CHECKBOX_OPTION( $name, $value, " $tag", id => "${name}_$i", - aria_label => $label . 'option ' . ($i + 1) . ' ', + aria_label => $label . main::maketext('option [_1] ', $i + 1), %options ) ); } else { - push(@checks, main::NAMED_ANS_CHECKBOX($name, $value, " $tag", $extend, %options)); + push( + @checks, + main::NAMED_ANS_CHECKBOX( + $name, $value, " $tag", $extend, %options, + aria_label => $label . main::maketext('option [_1] ', 1) + ) + ); } } diff --git a/macros/parsers/parserPopUp.pl b/macros/parsers/parserPopUp.pl index b1ae02f99..b8b9dab93 100644 --- a/macros/parsers/parserPopUp.pl +++ b/macros/parsers/parserPopUp.pl @@ -419,7 +419,7 @@ sub MENU { my $menu = ""; main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name; my $answer_value = (defined($main::inputs_ref->{$name}) ? $main::inputs_ref->{$name} : ''); - my $aria_label = main::generate_aria_label($name); + my $aria_label = $options{aria_label} // main::generate_aria_label($name); if ($main::displayMode =~ m/^HTML/) { if ($self->{useHTMLSelect}) { diff --git a/macros/parsers/parserRadioButtons.pl b/macros/parsers/parserRadioButtons.pl index e3b7ccb39..a74336f81 100644 --- a/macros/parsers/parserRadioButtons.pl +++ b/macros/parsers/parserRadioButtons.pl @@ -627,7 +627,6 @@ sub BUTTONS { my @choices = @{ $self->{orderedChoices} }; my @radio = (); main::RECORD_IMPLICIT_ANS_NAME($name = main::NEW_ANS_NAME()) unless $name; - my $label = main::generate_aria_label($name); foreach my $i (0 .. $#choices) { my $value = $self->{values}[$i]; @@ -639,8 +638,7 @@ sub BUTTONS { @radio, main::NAMED_ANS_RADIO_EXTENSION( $name, $value, $tag, - aria_label => $label . "option " . ($i + 1) . " ", - id => "${name}_$i", + id => "${name}_$i", $self->{uncheckable} ? ( attributes => { diff --git a/macros/parsers/parserRadioMultiAnswer.pl b/macros/parsers/parserRadioMultiAnswer.pl index a8f4c0c09..c648e50a4 100644 --- a/macros/parsers/parserRadioMultiAnswer.pl +++ b/macros/parsers/parserRadioMultiAnswer.pl @@ -620,6 +620,34 @@ sub label { return $self->{labels}[$i]; } +sub generate_aria_label { + my ($name, $radioIndex, $partIndex) = @_; + my $label = ''; + + $name =~ s/$answerPrefix//; + + # Check for the quiz prefix. + if ($name =~ /^Q\d+/ || $name =~ /^MaTrIx_Q\d+/) { + $name =~ s/Q0*(\d+)_//; + $label .= main::maketext('problem [_1] ', $1); + } + + # Get the answer number. + $name =~ /AnSwEr0*(\d+)/; + $label .= main::maketext('answer [_1] ', $1); + + $label .= main::maketext('part [_1] ', $radioIndex); + $label .= main::maketext('subpart [_1] ', $partIndex); + + # Check for a Matrix answer. + if ($name =~ /^MaTrIx_/) { + $name =~ /_(\d+)_(\d+)$/; + $label .= main::maketext('row [_1] column [_2] ', $1 + 1, $2 + 1); + } + + return $label; +} + # Produce the answer rule. sub ans_rule { my ($self, $size, @options) = @_; @@ -648,6 +676,7 @@ sub ans_rule { ? (defined $size->[$i][ $_ - 1 ] ? $size->[$i][ $_ - 1 ] : 20) : $size, answer_group_name => $radio_name, + aria_label => generate_aria_label($name, $i + 1, $_), @options ) ) @@ -663,6 +692,7 @@ sub ans_rule { ? (defined $size->[$i][ $_ - 1 ] ? $size->[$i][ $_ - 1 ] : 20) : $size, answer_group_name => $radio_name, + aria_label => generate_aria_label($name, $i + 1, $_), @options ) ) @@ -724,11 +754,10 @@ sub begin_radio { HTML => qq{
} . main::tag( 'input', - type => 'radio', - name => $name, - id => "$name$idSuffix", - aria_label => main::generate_aria_label("$answerPrefix${name}_0") . ' option ' . ($i + 1), - value => $value, + type => 'radio', + name => $name, + id => "$name$idSuffix", + value => $value, $self->{uncheckable} ? ( data_uncheckable_radio => 1,