Remove dangling label for= on 22 more styles-page headings - #3498
vivi-the-going-merry[bot] wants to merge 1 commit into
Conversation
Follow-up to #3366/#3463 (formidable-pro#6709): FrmSliderStyleComponent's template always puts the caller's 'id' on a type="hidden" storage input, never on a visible/labelable control, so any for="<that id>" heading is dangling by construction. FrmAlignStyleComponent silently discards 'id' entirely (same defect, different flavor). 20 slider-based instances across 7 files not touched by #3463 (_buttons.php alone has 7), matching Franky's Sept 19 DOM-confirmed count on #3366. Plus 2 more Align-based instances (_field-description.php, _field-labels.php) outside that slider-only scope, same root cause. Fix matches established precedent exactly: drop the dangling for=, keep 'id' on slider calls (still used by js/admin/style.js's getElementById lookup on the hidden input), drop 'id' on the two Align calls (confirmed unused anywhere in js/).
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: Strategy11/formidable-forms/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
There was a problem hiding this comment.
Approve — verified directly in source, not just from the PR's own claims: for every removed for= (slider-based labels), the corresponding id is still present on the component's FrmSliderStyleComponent call and lands on the hidden <input type="hidden" id="..."> in templates/slider.php — a hidden input was never a valid for= target, so removing the dangling attribute is correct and doesn't touch anything JS relies on. For the two Align-based id removals (frm_description_align, frm_label_align), grepped js/ and confirmed neither string appears anywhere — safe to drop entirely, matching #3366/#3463's established precedent exactly (slider keeps id, align/direction drops id). Markup-only, no visual change, no JS/build files touched, no danger-zone/compat concerns. Matches the same pattern already approved twice on #3366/#3463.
Follow-up to #3366 / #3463 (Strategy11/formidable-pro#6709).
FrmSliderStyleComponent's template always puts the caller'sidon atype="hidden"storage input, never on a visible/labelable control, so anyfor="<that id>"heading is dangling by construction.FrmAlignStyleComponentsilently discards
identirely — same defect, different flavor. Confirmedby reading both component templates (
slider.php,align.php).This fixes 22 more instances not covered by #3366/#3463 (still open,
unmerged), across 8 files:
_buttons.php(7)_field-labels.php(4, including one Align-based)_field-description.php(3, including one Align-based)_field-colors.php(2)_form-messages.php(2)_form-title.php(2)_check-box-radio-fields.php(1)_form-description.php(1)20 of these are slider-based, matching Franky's Sept 19 DOM-confirmed count
of 26 broken pairs on #3366 (6 of the 26 are in
_field-sizes.php/_general.php, already covered by #3463). The other 2 (frm_description_align,frm_label_align) are Align-based, outside that slider-only sweep — sameroot cause, found while cross-referencing every
FrmSliderStyleComponent/FrmAlignStyleComponent/FrmDirectionStyleComponent/FrmFieldShapeStyleComponentcall site against remaining
for=attributes inclasses/views/styles/*.php.Fix matches the established precedent exactly: drop the dangling
for=;keep
'id'on Slider calls (still consumed by the hidden input and byjs/admin/style.js'sgetElementByIdlookup); drop'id'on the two Aligncalls (confirmed unused anywhere in
js/).Verified live, not just from source:
formidable-preview-env,playwright-cliDOM check on the Styles admin page — counted everylabel[for]whose target is missing or a hidden input. Master (red):all 22 targeted ids present as dangling. This branch (green): all 22
gone, label/problem counts drop by exactly 22 (151→129 labels, 49→27
problems), no regressions elsewhere. IBM's
checkIbmAccessibilitycan't gate this (
.assertCompliance(false)never fails CI) — sameknown gap noted on #3366/#3463.
New finding, not fixed here (different repo):
formidable-pro'sclasses/views/styles/_section-fields.phphas aFrmSliderStyleComponentcall with
'id' => 'frm_success_font_size'— reuses formidable-forms'own
_form-messages.phpid (a duplicate-DOM-id bug on top of the samedangling-label defect, likely should be
frm_section_font_sizeto matchthe field name
section_font_size). Flagging on the tracking issue ratherthan fixing here (wrong repo).
Markup-only, no visual change (same
frm-style-item-headinglabel text,same input controls) — no before/after screenshot needed, same reasoning
already accepted on #3366/#3463.
Refs Strategy11/formidable-pro#6709 (does not close it alone — #3366 and #3463 still need to merge too, see comment on the issue)