Follow-up from PR #3468's review (two non-blocking notes from the same round):
-
Every one of the 21 data-slider-label-for sites in classes/views/styles/ duplicates the decision to print a conditional for="..." as a per-site PHP conditional keyed off FrmSliderStyleComponent::is_value_measured(), when slider-component.js's initListeners() already resolves valueInput.disabled for every slider at page load via the same underlying is_measured_unit() logic (slider.php:132's disabled() call). A single JS init-time pass — find the label via the existing [data-slider-label-for="..."] marker and call updateLabelFocusTarget(valueInput, !valueInput.disabled) once per slider — would let every template keep a plain static for and never need the conditional at all.
-
Separately, each of the 21 sites hand-types the {id}-value string twice (once in data-slider-label-for, once in the conditional for), plus a third time in the FrmSliderStyleComponent constructor's 'id' option. A small static helper on FrmSliderStyleComponent (e.g. label_for_attrs( $id, $value ) printing both attributes from one $id) would cut the two adjacent literals in the <label> down to one, at all 21 sites. If going this route, note phpcs.xml:76's WordPress.Security.EscapeOutput.OutputNotEscaped sniff will need the helper allowlisted (or self-escaping) since it'd be echoed directly, same as FrmAppHelper::array_to_html_params() already handles elsewhere in this codebase.
Doing (1) first would make (2) unnecessary — the JS-side pass removes the PHP conditional entirely, so there'd be nothing left needing a helper. Worth deciding as one piece of work rather than building both.
Neither note blocks #3468 — Franky approved that PR; these are structural cleanup for a future pass.
Follow-up from PR #3468's review (two non-blocking notes from the same round):
Every one of the 21
data-slider-label-forsites inclasses/views/styles/duplicates the decision to print a conditionalfor="..."as a per-site PHP conditional keyed offFrmSliderStyleComponent::is_value_measured(), whenslider-component.js'sinitListeners()already resolvesvalueInput.disabledfor every slider at page load via the same underlyingis_measured_unit()logic (slider.php:132'sdisabled()call). A single JS init-time pass — find the label via the existing[data-slider-label-for="..."]marker and callupdateLabelFocusTarget(valueInput, !valueInput.disabled)once per slider — would let every template keep a plain staticforand never need the conditional at all.Separately, each of the 21 sites hand-types the
{id}-valuestring twice (once indata-slider-label-for, once in the conditionalfor), plus a third time in theFrmSliderStyleComponentconstructor's'id'option. A small static helper onFrmSliderStyleComponent(e.g.label_for_attrs( $id, $value )printing both attributes from one$id) would cut the two adjacent literals in the<label>down to one, at all 21 sites. If going this route, notephpcs.xml:76'sWordPress.Security.EscapeOutput.OutputNotEscapedsniff will need the helper allowlisted (or self-escaping) since it'd be echoed directly, same asFrmAppHelper::array_to_html_params()already handles elsewhere in this codebase.Doing (1) first would make (2) unnecessary — the JS-side pass removes the PHP conditional entirely, so there'd be nothing left needing a helper. Worth deciding as one piece of work rather than building both.
Neither note blocks #3468 — Franky approved that PR; these are structural cleanup for a future pass.