fix: honour setDisabled(true) on RadioInput - #128
Merged
Conversation
RadioInput::getControl() only ever asked isValueDisabled() per item, and that helper deliberately returns false when the control is disabled as a whole, so setDisabled(true) rendered a fully interactive radio list. Set the attribute on the fieldset container instead, the same way CheckboxListInput already handles a wholly disabled control — one attribute disables every radio inside it. Closes #112 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #128 +/- ##
============================================
+ Coverage 96.94% 97.05% +0.10%
Complexity 308 308
============================================
Files 25 25
Lines 983 984 +1
============================================
+ Hits 953 955 +2
+ Misses 30 29 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #112.
The bug
RadioInput::getControl()decided each item'sdisabledattribute purely fromisValueDisabled(), and that helper returnsfalsewhenever the control is disabled as a whole (it only inspects the array form). Nothing else consultedisControlDisabled(), sosetDisabled(true)on a radio list rendered markup with nodisabledanywhere — the list looked and behaved as fully interactive, contrary to thebool|arraysignature.Nette still refused the posted value server-side, so this was a rendering defect rather than a security hole, but the user-visible behaviour was wrong.
The fix
Set the attribute on the
fieldsetcontainer, whichRadioInputalready uses as its container prototype:This mirrors
CheckboxListInput, which puts'disabled' => $this->isControlDisabled()on its own fieldset — one attribute disables every control inside it, and per-item disabling viasetDisabled(['x'])keeps working unchanged.Tests
tests/Traits/ChoiceInputTraitTest.phpcarried a test that deliberately pinned this gap (testDisablingTheWholeRadioListIsNotReflectedInTheHtml) so a fix would surface as a failure. That test is replaced by one asserting the correct behaviour, plus:setDisabled(false)drops the attribute againshowValidation()tests/Inputs/RadioInputTest.phpfor both the wholly-disabled and single-item-disabled casestests/E2E/FormSubmissionTest.phpcovering render + rejected post togethermake tests(155 tests),make phpstanandmake csall pass.🤖 Generated with Claude Code