Stop adding aria invalid directly to radio button and checkbox inputs#2507
Conversation
WalkthroughAdds group-level Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Renderer as Form Renderer
participant FrmFieldFormHtml as FrmFieldFormHtml
participant FieldCheckbox as FrmFieldCheckbox
participant FieldRadio as FrmFieldRadio
participant BrowserJS as formidable.js
Note over User,Renderer: Render form or update validation state
User->>Renderer: Request render / submit
Renderer->>FrmFieldFormHtml: add_multiple_input_attributes(args)
alt field has validation errors
FrmFieldFormHtml->>FrmFieldFormHtml: Inject aria-invalid="true" on group container attributes
else no errors
FrmFieldFormHtml->>FrmFieldFormHtml: No aria-invalid on group
end
Note over Renderer,FieldCheckbox: Per-input attribute assembly
Renderer->>FieldCheckbox: set_aria_invalid_error(&shortcode_atts, args)
FieldCheckbox->>FieldCheckbox: Unset aria-invalid on individual checkbox inputs
Renderer->>FieldRadio: set_aria_invalid_error(&shortcode_atts, args)
FieldRadio->>FieldRadio: Unset aria-invalid on individual radio inputs
Note over BrowserJS: Runtime updates after validation
BrowserJS->>Renderer: add/remove field error
alt field is radio/checkbox
BrowserJS->>Renderer: set/clear aria-invalid on group ancestor
else other field type
BrowserJS->>Renderer: set/clear aria-invalid on input
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
classes/models/FrmFieldFormHtml.php (2)
534-537: Make attribute injection robust (replace role="group" or role="radiogroup")Current replacement fails if the template uses role="radiogroup". Prefer a regex that matches either.
Apply this diff:
- $this->html = str_replace( ' role="group"', $html_attributes, $this->html ); + $this->html = preg_replace( '/\srole="(?:radio)?group"/', $html_attributes, $this->html, 1 );
529-533: Optional: append error id via aria-describedby on the groupImproves SR announcement without relying on per-input aria-invalid. Safe for both group and radiogroup.
Example (append if present, else set):
if ( ! empty( $this->pass_args['errors'][ 'field' . $this->field_id ] ) ) { $error_id = 'frm_error_' . $this->html_id; if ( ! empty( $attributes['aria-describedby'] ) ) { $attributes['aria-describedby'] .= ' ' . $error_id; } else { $attributes['aria-describedby'] = $error_id; } }classes/models/fields/FrmFieldRadio.php (1)
112-124: Silence unused $args and replace placeholder @SInCEAvoid PHPMD UnusedFormalParameter and fill in the real version before merge.
Apply this diff to silence PHPMD:
public function set_aria_invalid_error( &$shortcode_atts, $args ) { + // Silence unused parameter from parent signature. + (void) $args; unset( $shortcode_atts['aria-invalid'] ); }And update
@since x.xto the release version for this change.classes/models/fields/FrmFieldCheckbox.php (1)
112-123: Silence unused $args and replace placeholder @SInCEMirror the radio fix to keep static analysis clean and docs accurate.
Apply this diff:
public function set_aria_invalid_error( &$shortcode_atts, $args ) { + // Silence unused parameter from parent signature. + (void) $args; unset( $shortcode_atts['aria-invalid'] ); }And update
@since x.xto the actual release version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/models/FrmFieldFormHtml.php(1 hunks)classes/models/fields/FrmFieldCheckbox.php(1 hunks)classes/models/fields/FrmFieldRadio.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
classes/models/fields/FrmFieldRadio.php (2)
classes/models/fields/FrmFieldCheckbox.php (1)
set_aria_invalid_error(121-123)classes/models/fields/FrmFieldType.php (1)
set_aria_invalid_error(1046-1048)
classes/models/fields/FrmFieldCheckbox.php (2)
classes/models/fields/FrmFieldRadio.php (1)
set_aria_invalid_error(122-124)classes/models/fields/FrmFieldType.php (1)
set_aria_invalid_error(1046-1048)
🪛 PHPMD (2.15.0)
classes/models/fields/FrmFieldRadio.php
122-122: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
classes/models/fields/FrmFieldCheckbox.php
121-121: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (2)
classes/models/fields/FrmFieldRadio.php (1)
112-124: Good: per-input aria-invalid removed for radios; handled at radiogroupThis aligns with ARIA and the new group-level handling.
classes/models/fields/FrmFieldCheckbox.php (1)
112-123: Good: per-input aria-invalid removed for checkboxes; group-level flow supportedMatches the radiogroup approach and the form HTML changes.
Crabcyborg
deleted the
stop_adding_aria_invalid_directly_to_radio_button_and_checkbox_inputs
branch
1 participant
Related ticket https://secure.helpscout.net/conversation/3085736983/238704
This fixes issues caught by SiteImprove's Accessibility Checker.
Pre-release
formidable-6.24.2b.zip
What's changing
Radio buttons and checkboxes will no longer include aria-invalid. It isn't meant for these inputs. Instead, the role="radiogroup" or role="group" parents will use the aria-invalid attribute.
When testing
You can download the SiteImprove browser widget. It has links for various browsers here https://www.siteimprove.com/why-siteimprove/integrations/browser-extensions/
It's good to test this with an Accessibility tool like Voiceover. We want an invalid group to still appear as invalid to a screen reader. I tested this with Voiceover and it works.
This is the error that would previous show:
