feat(form): Add ariaDescribedBySuffix property and update getAttributes method for improved accessibility support.#60
feat(form): Add ariaDescribedBySuffix property and update getAttributes method for improved accessibility support.#60terabytesoftw wants to merge 3 commits intomainfrom
ariaDescribedBySuffix property and update getAttributes method for improved accessibility support.#60Conversation
…butes` method for improved accessibility support.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds configurable aria-describedby suffix support to form element builders and updates tests to stop assuming default id emission; introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/Form/InputResetTest.php (1)
422-431:⚠️ Potential issue | 🟡 MinorDefault-values test should not force an id.
This test is named for default rendering but now sets an explicit id, which duplicates
testRenderWithIdand conflicts with the “no default id” contract. Consider reverting to the id-less expectation.🛠️ Suggested adjustment
- <input id="value" type="reset"> + <input type="reset"> @@ - InputReset::tag() - ->id('value') - ->render(), + InputReset::tag()->render(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Form/InputResetTest.php` around lines 422 - 431, The test testRenderWithDefaultValues currently forces an id by calling InputReset::tag()->id('value')->render(), which duplicates testRenderWithId and violates the contract that default rendering should not include an id; change the assertion to expect no id and remove the explicit ->id('value') call (leave InputReset::tag()->render() as the invocation) so the output matches the id-less default rendering checked by testRenderWithDefaultValues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Form/Button.php`:
- Around line 70-83: Add a unit test that ensures
Button::ariaDescribedBySuffix(string) is immutable: instantiate a Button, call
ariaDescribedBySuffix('x') and assert the returned object is not the same
instance as the original and that the original's ariaDescribedBySuffix value
remains unchanged while the new instance has the updated value; place this in
the Button test suite (e.g., ButtonTest::testAriaDescribedBySuffixIsImmutable)
and use identity (!== / assertNotSame) and value assertions to catch accidental
removal of the clone in ariaDescribedBySuffix().
In `@src/Form/Form.php`:
- Around line 78-86: The current logic in Form.php uses the raw $id from
getAttribute('id') when composing aria-describedby and can produce values like
"-help" for empty or whitespace-only IDs; before composing (where $id is used
with $ariaDescribedBySuffix and assigned to $attributes['aria-describedby']),
normalize and guard $id by treating empty or whitespace-only strings as null
(e.g., trim and check === ''), then only set aria-describedby when $id is
non-null/non-empty; update the code paths around the $id variable, the
ariaDescribedBySuffix computation, and the block that assigns
$attributes['aria-describedby'] so empty IDs do not produce invalid descriptors.
In `@src/Form/TextArea.php`:
- Around line 67-80: Add an immutability unit test for
TextArea::ariaDescribedBySuffix that asserts the method returns a new instance
and does not mutate the original: create an initial TextArea, call
ariaDescribedBySuffix('...') to get $new, assert $new !== $original, assert the
original still has the old ariaDescribedBySuffix value and the $new instance has
the updated value; this will prevent regressions where the method stops cloning
and mutates state.
In `@tests/Form/FormTest.php`:
- Around line 304-318: Add two assertions for ariaDescribedBySuffix: (1) add a
new test that verifies the empty-suffix fallback produces the expected output
when calling ariaDescribedBySuffix('') (or null) together with
addAriaAttribute('describedby', true) and id('form') to ensure the attribute
still renders with the default/empty suffix; (2) in the existing
testReturnNewInstanceWhenSettingAttribute() add an immutability assertion using
assertNotSame($form, $form->ariaDescribedBySuffix('help')) to confirm
ariaDescribedBySuffix returns a new Form instance; reference the fluent setter
ariaDescribedBySuffix and the builder methods addAriaAttribute and id to locate
where to add these checks.
---
Outside diff comments:
In `@tests/Form/InputResetTest.php`:
- Around line 422-431: The test testRenderWithDefaultValues currently forces an
id by calling InputReset::tag()->id('value')->render(), which duplicates
testRenderWithId and violates the contract that default rendering should not
include an id; change the assertion to expect no id and remove the explicit
->id('value') call (leave InputReset::tag()->render() as the invocation) so the
output matches the id-less default rendering checked by
testRenderWithDefaultValues.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (29)
CHANGELOG.mdsrc/Form/Button.phpsrc/Form/Form.phpsrc/Form/TextArea.phptests/Form/ButtonTest.phptests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputColorTest.phptests/Form/InputDateTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/InputHiddenTest.phptests/Form/InputImageTest.phptests/Form/InputMonthTest.phptests/Form/InputNumberTest.phptests/Form/InputPasswordTest.phptests/Form/InputRadioTest.phptests/Form/InputRangeTest.phptests/Form/InputResetTest.phptests/Form/InputSearchTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputTextTest.phptests/Form/InputTimeTest.phptests/Form/InputUrlTest.phptests/Form/InputWeekTest.phptests/Form/TextAreaTest.phptests/Provider/Form/CheckedProvider.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: linter / Super Linter
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2026-02-22T19:26:33.769Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Sectioning/AsideTest.php:821-833
Timestamp: 2026-02-22T19:26:33.769Z
Learning: In the ui-awesome/html repository, do not use import aliases in PHP. When there are naming conflicts (e.g., two different Message classes), prefer the fully qualified class name (FQCN) such as \UIAwesome\Html\Attribute\Exception\Message::ATTRIBUTE_INVALID_VALUE instead of creating an alias like "use UIAwesome\Html\Attribute\Exception\Message as AttributeMessage". This should apply to PHP files across the codebase (not just tests).
Applied to files:
src/Form/Button.phptests/Provider/Form/CheckedProvider.phpsrc/Form/Form.phptests/Form/FormTest.phpsrc/Form/TextArea.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputRadioTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phptests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-09T11:28:09.334Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 35
File: tests/Provider/Form/CheckedProvider.php:13-14
Timestamp: 2026-02-09T11:28:09.334Z
Learning: In the ui-awesome/html repository, using `str_replace` to adapt shared data provider HTML strings for different input types (e.g., adapting `CheckedProvider` for both `InputCheckboxTest` and `InputRadioTest` by replacing "inputcheckbox-" with "inputradio-" and "checkbox" with "radio") is acceptable and considered clear. Don't suggest refactoring such patterns to avoid string replacement.
Applied to files:
tests/Provider/Form/CheckedProvider.phptests/Form/InputCheckboxTest.phptests/Form/ButtonTest.php
📚 Learning: 2026-02-06T21:37:44.509Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 24
File: tests/Form/InputTextTest.php:33-620
Timestamp: 2026-02-06T21:37:44.509Z
Learning: In the ui-awesome/html repository, prefer individual test methods over PHPUnit data providers in test classes. This helps maintainers see exactly how each test behaves without cross-referencing data providers. Apply to all PHP test files under tests. Use separate, descriptively named test methods to cover different inputs/edge cases; retain data providers only if they clearly improve readability or reduce duplication.
Applied to files:
tests/Provider/Form/CheckedProvider.phptests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputRadioTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phptests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-11T14:56:18.277Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 37
File: tests/Form/InputTextTest.php:448-457
Timestamp: 2026-02-11T14:56:18.277Z
Learning: In PHP test files under the repository ui-awesome/html (e.g., tests/Form/InputTextTest.php), it is acceptable to use short variable names (e.g., 2-character names like $id). PHPMD's short-variable rule does not need to be strictly followed in test code. During reviews, allow short names in test files and note that this relaxed style applies to tests, while production code should adhere to the standard variable naming conventions.
Applied to files:
tests/Provider/Form/CheckedProvider.phptests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputRadioTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phptests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-22T19:37:55.152Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:37:55.152Z
Learning: In PHP test suites for the ui-awesome/html repository, add explicit tests in each element's test file to validate InvalidArgumentException for trait-based attributes (e.g., HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.). Even if these traits are covered in the html-attribute package, ensure coverage is visible in the element tests because test knowledge may be lost when relying solely on trait package tests. Treat this as a guideline that should be applied broadly across element tests to confirm correct behavior and improve maintainability.
Applied to files:
tests/Provider/Form/CheckedProvider.phptests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputRadioTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phptests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-21T01:17:40.877Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 51
File: tests/List/UlTest.php:282-283
Timestamp: 2026-02-21T01:17:40.877Z
Learning: In the ui-awesome/html repository, for PHP tests, ensure that assertion messages in tests/List/UlTest.php for methods named testRenderWith*UsingEnum remain identical to the corresponding non-enum tests (i.e., do not append 'using enum' to the failure message). The reason: the underlying render method treats enum and string inputs the same, so failure output should be uniform. Apply this pattern to all similar test cases under tests/ to maintain consistent failure messages across enum vs non-enum variants.
Applied to files:
tests/Provider/Form/CheckedProvider.phptests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputRadioTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phptests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-22T19:29:56.689Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/List/DdTest.php:751-847
Timestamp: 2026-02-22T19:29:56.689Z
Learning: In the ui-awesome/html repository, tests for InvalidArgumentException validation of trait-based attributes (from html-attribute package) should be included in element test files even though the underlying trait behavior is tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite for AI analysis and developer reference.
Applied to files:
tests/Form/FormTest.phptests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/TextAreaTest.phptests/Form/InputTextTest.phptests/Form/InputDateTimeLocalTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phpCHANGELOG.mdtests/Form/InputResetTest.phptests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputImageTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-22T20:57:07.467Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 59
File: tests/Form/FormTest.php:980-1132
Timestamp: 2026-02-22T20:57:07.467Z
Learning: In the ui-awesome/html repository, the `HasAutocomplete` trait (from the html-attribute package) does not validate input values and accepts any string without throwing InvalidArgumentException. Therefore, no `testThrowInvalidArgumentExceptionWhenSettingAutocomplete` test should be added. Only traits that use `Validator::oneOf()` (like HasEnctype, HasMethod, etc.) require InvalidArgumentException tests.
Applied to files:
tests/Form/InputCheckboxTest.phptests/Form/InputMonthTest.phptests/Form/InputUrlTest.phptests/Form/InputDateTest.phptests/Form/InputRangeTest.phptests/Form/InputEmailTest.phptests/Form/InputFileTest.phptests/Form/InputTextTest.phptests/Form/InputWeekTest.phptests/Form/InputTimeTest.phptests/Form/InputHiddenTest.phpCHANGELOG.mdtests/Form/ButtonTest.phptests/Form/InputNumberTest.phptests/Form/InputSubmitTest.phptests/Form/InputTelTest.phptests/Form/InputSearchTest.phptests/Form/InputPasswordTest.phptests/Form/InputColorTest.php
📚 Learning: 2026-02-22T19:38:01.916Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:38:01.916Z
Learning: In the ui-awesome/html repository, the maintainer prefers to include InvalidArgumentException validation tests for trait-based attributes (HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.) in each element's test file, even though these traits are tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite because "test knowledge is lost" when relying solely on trait package tests—this explicit coverage is considered "a skill for AI" analysis and developer reference.
Applied to files:
CHANGELOG.md
🧬 Code graph analysis (21)
src/Form/Button.php (2)
src/Form/Form.php (2)
ariaDescribedBySuffix(59-65)getAttributes(74-89)src/Form/TextArea.php (2)
ariaDescribedBySuffix(74-80)getAttributes(89-104)
src/Form/Form.php (2)
src/Form/Button.php (2)
ariaDescribedBySuffix(77-83)getAttributes(92-107)src/Form/TextArea.php (2)
ariaDescribedBySuffix(74-80)getAttributes(89-104)
tests/Form/FormTest.php (3)
src/Form/Form.php (2)
Form(32-102)ariaDescribedBySuffix(59-65)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
src/Form/TextArea.php (2)
src/Form/Button.php (2)
ariaDescribedBySuffix(77-83)getAttributes(92-107)src/Form/Form.php (2)
ariaDescribedBySuffix(59-65)getAttributes(74-89)
tests/Form/InputMonthTest.php (4)
src/Form/InputMonth.php (1)
InputMonth(52-97)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputUrlTest.php (4)
src/Form/InputUrl.php (1)
InputUrl(46-94)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputRadioTest.php (4)
tests/Form/InputUrlTest.php (2)
testRenderWithAriaDescribedByCustomSuffix(294-307)testRenderWithAttributes(309-320)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputDateTest.php (4)
src/Form/InputDate.php (1)
InputDate(51-96)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputRangeTest.php (4)
src/Form/InputRange.php (1)
InputRange(48-91)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputFileTest.php (5)
src/Form/InputFile.php (1)
InputFile(42-120)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)tests/Form/InputDateTest.php (1)
testRenderWithAriaDescribedBySuffixAndEmptySuffixUsesDefault(310-323)
tests/Form/TextAreaTest.php (3)
src/Form/TextArea.php (2)
TextArea(42-117)ariaDescribedBySuffix(74-80)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)
tests/Form/InputDateTimeLocalTest.php (3)
src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)tests/Form/InputDateTest.php (1)
testRenderWithAriaDescribedBySuffixAndEmptySuffixUsesDefault(310-323)
tests/Form/InputWeekTest.php (3)
src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputTimeTest.php (5)
tests/Form/InputUrlTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)src/Form/InputTime.php (1)
InputTime(51-96)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputHiddenTest.php (7)
src/Form/InputHidden.php (1)
InputHidden(31-68)tests/Form/InputTelTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(292-305)tests/Form/InputTextTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(295-308)tests/Form/InputUrlTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputResetTest.php (5)
tests/Form/InputTelTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(292-305)tests/Form/InputUrlTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputNumberTest.php (6)
tests/Form/InputTextTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(295-308)tests/Form/InputTimeTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)tests/Form/InputUrlTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputImageTest.php (9)
tests/Form/InputFileTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(305-318)tests/Form/InputTelTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(292-305)tests/Form/InputTextTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(295-308)tests/Form/InputTimeTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)tests/Form/InputUrlTest.php (1)
testRenderWithAriaDescribedByCustomSuffix(294-307)src/Form/InputImage.php (1)
InputImage(41-86)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputSubmitTest.php (4)
src/Form/InputSubmit.php (1)
InputSubmit(37-80)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputSearchTest.php (4)
src/Form/InputSearch.php (1)
InputSearch(49-98)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
tests/Form/InputColorTest.php (3)
src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)src/Form/TextArea.php (1)
ariaDescribedBySuffix(74-80)
🪛 GitHub Check: mutation / PHP 8.5-ubuntu-latest
src/Form/Button.php
[warning] 79-79:
Escaped Mutant for Mutator "CloneRemoval":
@@ @@
*/
public function ariaDescribedBySuffix(string $value): static
{
-
$new = clone $this;
-
$new = $this; $new->ariaDescribedBySuffix = $value; return $new;
src/Form/Form.php
[warning] 61-61:
Escaped Mutant for Mutator "CloneRemoval":
@@ @@
*/
public function ariaDescribedBySuffix(string $value): static
{
-
$new = clone $this;
-
$new = $this; $new->ariaDescribedBySuffix = $value; return $new;
src/Form/TextArea.php
[warning] 76-76:
Escaped Mutant for Mutator "CloneRemoval":
@@ @@
*/
public function ariaDescribedBySuffix(string $value): static
{
-
$new = clone $this;
-
$new = $this; $new->ariaDescribedBySuffix = $value; return $new;
🪛 PHPMD (2.15.0)
src/Form/Button.php
[warning] 68-68: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 97-97: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 100-100: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
src/Form/Form.php
[warning] 50-50: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 79-79: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 82-82: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
src/Form/TextArea.php
[warning] 65-65: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 94-94: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 97-97: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
tests/Form/InputCheckboxTest.php
[error] 819-819: Avoid using static access to class '\UIAwesome\Html\Form\InputCheckbox' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputMonthTest.php
[error] 903-903: Avoid using static access to class '\UIAwesome\Html\Form\InputMonth' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputUrlTest.php
[error] 917-917: Avoid using static access to class '\UIAwesome\Html\Form\InputUrl' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputRadioTest.php
[error] 813-813: Avoid using static access to class '\UIAwesome\Html\Form\InputRadio' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputDateTest.php
[error] 922-922: Avoid using static access to class '\UIAwesome\Html\Form\InputDate' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputRangeTest.php
[error] 876-876: Avoid using static access to class '\UIAwesome\Html\Form\InputRange' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputEmailTest.php
[error] 943-943: Avoid using static access to class '\UIAwesome\Html\Form\InputEmail' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputFileTest.php
[error] 837-837: Avoid using static access to class '\UIAwesome\Html\Form\InputFile' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputTextTest.php
[error] 931-931: Avoid using static access to class '\UIAwesome\Html\Form\InputText' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputDateTimeLocalTest.php
[error] 918-918: Avoid using static access to class '\UIAwesome\Html\Form\InputDateTimeLocal' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputWeekTest.php
[error] 905-905: Avoid using static access to class '\UIAwesome\Html\Form\InputWeek' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputTimeTest.php
[error] 905-905: Avoid using static access to class '\UIAwesome\Html\Form\InputTime' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputHiddenTest.php
[error] 755-755: Avoid using static access to class '\UIAwesome\Html\Form\InputHidden' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputResetTest.php
[error] 759-759: Avoid using static access to class '\UIAwesome\Html\Form\InputReset' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputNumberTest.php
[error] 916-916: Avoid using static access to class '\UIAwesome\Html\Form\InputNumber' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputImageTest.php
[error] 875-875: Avoid using static access to class '\UIAwesome\Html\Form\InputImage' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputSubmitTest.php
[error] 875-875: Avoid using static access to class '\UIAwesome\Html\Form\InputSubmit' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputTelTest.php
[error] 916-916: Avoid using static access to class '\UIAwesome\Html\Form\InputTel' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputSearchTest.php
[error] 928-928: Avoid using static access to class '\UIAwesome\Html\Form\InputSearch' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputPasswordTest.php
[error] 916-916: Avoid using static access to class '\UIAwesome\Html\Form\InputPassword' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
tests/Form/InputColorTest.php
[error] 868-868: Avoid using static access to class '\UIAwesome\Html\Form\InputColor' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
🔇 Additional comments (51)
CHANGELOG.md (1)
33-48: Changelog entry reflects the new ariaDescribedBySuffix feature.
Looks consistent with the PR scope.tests/Provider/Form/CheckedProvider.php (1)
35-299: Provider expectations align with the no-id default rendering.
The updated HTML strings now match the revised output contract.tests/Form/InputHiddenTest.php (2)
68-229: Updated render expectations align with the no-id default behavior.
The refreshed outputs keep attribute coverage intact while dropping implicit ids.Also applies to: 305-821
290-303: Custom ariaDescribedBySuffix coverage looks good.
Confirms suffix concatenation with the explicit id when describedby=true.tests/Form/InputImageTest.php (2)
69-231: Render expectation updates match the no-id defaults.
Attribute coverage remains consistent with the revised rendering behavior.Also applies to: 319-941
304-317: Custom ariaDescribedBySuffix test is a solid addition.
Confirms suffix handling alongside explicit ids.tests/Form/InputPasswordTest.php (2)
71-233: Updated expectations align with no-id rendering defaults.
Outputs remain consistent across attribute combinations.Also applies to: 308-981
293-306: Custom ariaDescribedBySuffix coverage looks correct.
Validates the suffix integration when describedby is true.tests/Form/InputFileTest.php (2)
69-303: Expectations updated consistently with the no-id default rendering.
Coverage remains comprehensive across attribute combinations.Also applies to: 335-999
305-333: AriaDescribedBySuffix tests cover both custom and empty suffix fallback.
Great to see default “-help” behavior explicitly locked in.tests/Form/InputRadioTest.php (2)
74-294: Render expectation updates match the new no-id defaults.
The test suite still covers the same attribute permutations effectively.Also applies to: 311-875
296-309: Custom ariaDescribedBySuffix test is well-targeted.
Confirms correct suffix behavior with describedby=true.tests/Form/InputCheckboxTest.php (2)
72-293: Updated expectations align with the no-id rendering contract.
Attribute coverage remains thorough after the id removal.Also applies to: 324-883
294-323: Custom and empty-suffix ariaDescribedBySuffix tests look solid.
They clearly verify both custom and default suffix behavior.tests/Form/InputTelTest.php (1)
292-305: Covers customaria-describedbysuffix behavior.The new test clearly validates the custom suffix path and aligns with the updated accessibility API.
tests/Form/InputRangeTest.php (1)
291-304: Good coverage for custom suffix rendering.This test nicely locks in the expected
aria-describedbyvalue when a custom suffix is provided.tests/Form/InputUrlTest.php (1)
294-307: Custom suffix scenario is well-covered.The assertion captures the new suffix behavior precisely.
tests/Form/InputColorTest.php (1)
308-336: Nice coverage of both custom and default suffix paths.Including the empty-suffix fallback test strengthens the ARIA behavior contract.
tests/Form/InputMonthTest.php (1)
291-304: Custom suffix test looks solid.This locks in the expected
aria-describedbyvalue when suffix customization is used.tests/Form/InputTimeTest.php (1)
294-307: Custom suffix behavior is well-captured.The assertion precisely validates the suffix-based described-by output.
tests/Form/InputWeekTest.php (1)
71-969: LGTM — updated render expectations and custom suffix coverage.The expected outputs (no default
id) and the customariaDescribedBySuffixtest align with the new described-by behavior.tests/Form/InputEmailTest.php (1)
70-1008: LGTM — custom/empty suffix coverage and updated outputs look consistent.tests/Form/InputDateTimeLocalTest.php (1)
69-983: LGTM — describedby suffix tests and render expectations look consistent.tests/Form/InputDateTest.php (1)
71-986: LGTM — custom/empty suffix coverage and updated outputs look consistent.tests/Form/InputSearchTest.php (1)
70-992: LGTM — updated outputs and custom suffix coverage look consistent.tests/Form/TextAreaTest.php (3)
136-195: Good coverage for addAriaAttribute describedby variants.Covers explicit string, boolean true, string "true", and id-null cases cleanly.
244-288: Aria attributes + custom suffix coverage looks solid.Nice to see describedby through
ariaAttributes()and custom suffix behavior validated.
304-332: Attributes-path describedby tests look complete.Covers both boolean and string "true" inputs consistently.
tests/Form/ButtonTest.php (2)
28-28: LGTM: describedby + prefix/suffix coverage is clear.Inline import is appropriate for the prefix/suffix render checks.
Also applies to: 132-229
275-357: LGTM: ariaAttributes/attributes describedby paths are covered.The custom suffix test is a good addition.
tests/Form/InputTextTest.php (5)
72-309: LGTM: updated render expectations + custom suffix test.The describedby cases and custom suffix test align with the new rendering contract.
310-461: LGTM: attributes/default-values expectations updated consistently.
464-709: LGTM: global attribute and element-specific render checks updated.
711-897: LGTM: mutation/removal and template/theme scenarios remain consistent.
912-997: LGTM: title/toString/translate/user-overrides/value coverage is consistent.tests/Form/InputResetTest.php (5)
68-203: LGTM: describedby boolean/string coverage is comprehensive.
205-344: LGTM: custom suffix + attributes-path describedby tests look good.
346-575: LGTM: global attributes and id/lang/name expectations updated consistently.
578-723: LGTM: removal/mutation scenarios are consistent with the new defaults.
727-825: LGTM: theme, toString, translate, and value paths look consistent.tests/Form/InputSubmitTest.php (5)
69-204: LGTM: describedby cases and prefix/suffix rendering are well covered.
206-345: LGTM: custom suffix and attributes-path describedby tests look good.
347-432: LGTM: defaults/autofocus/class expectations updated consistently.
434-705: LGTM: global attributes, form-related, and removal paths updated.
750-941: LGTM: role/style/template/theme/toString/translate/value remain consistent.tests/Form/InputNumberTest.php (6)
70-205: LGTM: describedby boolean/string cases and prefix/suffix rendering covered.
207-346: LGTM: custom suffix + attributes-path describedby tests are solid.
348-459: LGTM: autocomplete/autofocus/class/defaults expectations updated.
461-707: LGTM: global attributes, constraints, and removal paths consistent.
752-919: LGTM: required/role/template/theme/toString coverage is consistent.
921-979: LGTM: translate/user-overrides/value expectations align with new defaults.
| /** @phpstan-var string|null $id */ | ||
| $id = $this->getAttribute('id', null); | ||
| $ariaDescribedBy = $this->getAttribute('aria-describedby', null); | ||
|
|
||
| $ariaDescribedBySuffix = $this->ariaDescribedBySuffix === '' ? '-help' : "-{$this->ariaDescribedBySuffix}"; | ||
|
|
||
| if ($ariaDescribedBy === true || $ariaDescribedBy === 'true') { | ||
| $attributes['aria-describedby'] = $id !== null ? "{$id}{$ariaDescribedBySuffix}" : null; | ||
| } |
There was a problem hiding this comment.
Guard against empty IDs when composing aria-describedby.
If id is an empty/whitespace string, the current logic still emits aria-describedby="-help", which won’t point to a real element. Treat empty strings as missing before composing the value.
🛠️ Suggested fix
/** `@phpstan-var` string|null $id */
$id = $this->getAttribute('id', null);
+ $id = is_string($id) && trim($id) === '' ? null : $id;🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 79-79: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 82-82: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Form/Form.php` around lines 78 - 86, The current logic in Form.php uses
the raw $id from getAttribute('id') when composing aria-describedby and can
produce values like "-help" for empty or whitespace-only IDs; before composing
(where $id is used with $ariaDescribedBySuffix and assigned to
$attributes['aria-describedby']), normalize and guard $id by treating empty or
whitespace-only strings as null (e.g., trim and check === ''), then only set
aria-describedby when $id is non-null/non-empty; update the code paths around
the $id variable, the ariaDescribedBySuffix computation, and the block that
assigns $attributes['aria-describedby'] so empty IDs do not produce invalid
descriptors.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 175 193 +18
===========================================
Files 94 94
Lines 370 406 +36
===========================================
+ Hits 370 406 +36 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/Form/Form.php (1)
78-85:⚠️ Potential issue | 🟡 MinorHandle empty/whitespace
idbefore generatingaria-describedby.The current null-only guard still allows outputs like
-helpwhenidis blank.🛠️ Suggested fix
/** `@phpstan-var` string|null $id */ $id = $this->getAttribute('id', null); +$id = is_string($id) && trim($id) === '' ? null : $id; $ariaDescribedBy = $this->getAttribute('aria-describedby', null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Form/Form.php` around lines 78 - 85, The guard only checks for null so a blank or whitespace-only id still produces "-help"; update the logic in Form.php where $id is retrieved (via $this->getAttribute('id', null')) and where aria-describedby is built (using $ariaDescribedBy, $ariaDescribedBySuffix) to ensure $id is non-empty after trimming before concatenation — i.e., treat empty or whitespace-only ids as missing and only set $attributes['aria-describedby'] when trim((string)$id) !== '' (or equivalent) so you don't emit just the suffix.tests/Form/TextAreaTest.php (1)
274-288: 🧹 Nitpick | 🔵 TrivialAdd missing fallback/immutability coverage for
ariaDescribedBySuffix()inTextAreaTest.Custom suffix behavior is covered, but the empty suffix default path and fluent immutability assertion are still untested.
✅ Suggested test additions
+ public function testRenderWithAriaDescribedBySuffixAndEmptySuffixUsesDefault(): void + { + self::assertSame( + <<<HTML + <textarea id="textarea" aria-describedby="textarea-help"> + </textarea> + HTML, + TextArea::tag() + ->addAriaAttribute('describedby', true) + ->ariaDescribedBySuffix('') + ->id('textarea') + ->render(), + "Failed asserting that an empty 'ariaDescribedBySuffix()' falls back to '-help'.", + ); + }public function testReturnNewInstanceWhenSettingAttribute(): void { $textArea = TextArea::tag(); + self::assertNotSame( + $textArea, + $textArea->ariaDescribedBySuffix('help'), + 'Should return a new instance when setting the attribute, ensuring immutability.', + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Form/TextAreaTest.php` around lines 274 - 288, Add two assertions to TextAreaTest: one verifying that calling ariaDescribedBySuffix('') (or not setting a suffix) falls back to the default suffix behavior so the rendered aria-describedby uses the base id (e.g., "textarea") rather than a custom suffix, and another asserting immutability by checking that ariaDescribedBySuffix() returns a new TextArea instance (fluent API) rather than mutating the original—use TextArea::tag(), addAriaAttribute('describedby', true), ariaDescribedBySuffix(...), id('textarea'), render() and compare outputs or instance identity to validate the fallback and that the original object is unchanged.tests/Form/FormTest.php (1)
304-318: 🧹 Nitpick | 🔵 TrivialAdd regression tests for empty-suffix fallback and fluent immutability.
You covered custom suffix, but not the
ariaDescribedBySuffix('')fallback path, and there is still noassertNotSameforariaDescribedBySuffix()intestReturnNewInstanceWhenSettingAttribute().✅ Suggested test additions
+ public function testRenderWithAriaDescribedBySuffixAndEmptySuffixUsesDefault(): void + { + self::assertSame( + <<<HTML + <form id="form" aria-describedby="form-help"> + </form> + HTML, + Form::tag() + ->addAriaAttribute('describedby', true) + ->ariaDescribedBySuffix('') + ->id('form') + ->render(), + "Failed asserting that an empty 'ariaDescribedBySuffix()' falls back to '-help'.", + ); + }public function testReturnNewInstanceWhenSettingAttribute(): void { $form = Form::tag(); + self::assertNotSame( + $form, + $form->ariaDescribedBySuffix('help'), + 'Should return a new instance when setting the attribute, ensuring immutability.', + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Form/FormTest.php` around lines 304 - 318, Add two assertions: one test to cover the empty-suffix fallback by calling ariaDescribedBySuffix('') on a Form::tag() (with addAriaAttribute('describedby', true) and id('form')) and asserting the rendered output matches the expected fallback behavior; and update the existing testReturnNewInstanceWhenSettingAttribute to include assertNotSame to verify fluent immutability when calling ariaDescribedBySuffix() (i.e., ensure the original Form instance is not the same as the returned instance). Use the existing methods Form::tag(), addAriaAttribute(), ariaDescribedBySuffix(), id(), render(), and the test names testRenderWithAriaDescribedByCustomSuffix and testReturnNewInstanceWhenSettingAttribute to locate where to add these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Form/Button.php`:
- Around line 96-103: The code composes aria-describedby using $id without
normalizing empty/whitespace IDs, which produces invalid values like "-help";
update the logic around getAttribute('id', null) so $id is normalized (trimmed)
and treated as null when it is an empty string or only whitespace before
building $ariaDescribedBySuffix and before setting
$attributes['aria-describedby'] in the branch that checks $ariaDescribedBy;
adjust the condition that assigns "{$id}{$ariaDescribedBySuffix}" to use the
normalized $id (or yield null) and keep references to getAttribute, $id,
$ariaDescribedBySuffix and the aria-describedby assignment to locate and change
the code.
In `@src/Form/TextArea.php`:
- Around line 93-100: The code builds aria-describedby when $ariaDescribedBy is
truthy but treats an empty string id as valid, producing invalid values like
"-help"; update the guard in TextArea.php to treat blank ids as null before
composing the suffix: ensure $id from getAttribute('id', null) is normalized
(e.g., trim and set to null if empty) or add a check before assigning
$attributes['aria-describedby'] so it only composes
"{$id}{$ariaDescribedBySuffix}" when $id is non-empty, using the existing
variables $id, $ariaDescribedBySuffix and method getAttribute to locate the
change.
---
Duplicate comments:
In `@src/Form/Form.php`:
- Around line 78-85: The guard only checks for null so a blank or
whitespace-only id still produces "-help"; update the logic in Form.php where
$id is retrieved (via $this->getAttribute('id', null')) and where
aria-describedby is built (using $ariaDescribedBy, $ariaDescribedBySuffix) to
ensure $id is non-empty after trimming before concatenation — i.e., treat empty
or whitespace-only ids as missing and only set $attributes['aria-describedby']
when trim((string)$id) !== '' (or equivalent) so you don't emit just the suffix.
In `@tests/Form/FormTest.php`:
- Around line 304-318: Add two assertions: one test to cover the empty-suffix
fallback by calling ariaDescribedBySuffix('') on a Form::tag() (with
addAriaAttribute('describedby', true) and id('form')) and asserting the rendered
output matches the expected fallback behavior; and update the existing
testReturnNewInstanceWhenSettingAttribute to include assertNotSame to verify
fluent immutability when calling ariaDescribedBySuffix() (i.e., ensure the
original Form instance is not the same as the returned instance). Use the
existing methods Form::tag(), addAriaAttribute(), ariaDescribedBySuffix(), id(),
render(), and the test names testRenderWithAriaDescribedByCustomSuffix and
testReturnNewInstanceWhenSettingAttribute to locate where to add these checks.
In `@tests/Form/TextAreaTest.php`:
- Around line 274-288: Add two assertions to TextAreaTest: one verifying that
calling ariaDescribedBySuffix('') (or not setting a suffix) falls back to the
default suffix behavior so the rendered aria-describedby uses the base id (e.g.,
"textarea") rather than a custom suffix, and another asserting immutability by
checking that ariaDescribedBySuffix() returns a new TextArea instance (fluent
API) rather than mutating the original—use TextArea::tag(),
addAriaAttribute('describedby', true), ariaDescribedBySuffix(...),
id('textarea'), render() and compare outputs or instance identity to validate
the fallback and that the original object is unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.mdsrc/Form/Button.phpsrc/Form/Form.phpsrc/Form/TextArea.phptests/Form/FormTest.phptests/Form/TextAreaTest.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: linter / Super Linter
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2026-02-22T19:26:33.769Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Sectioning/AsideTest.php:821-833
Timestamp: 2026-02-22T19:26:33.769Z
Learning: In the ui-awesome/html repository, do not use import aliases in PHP. When there are naming conflicts (e.g., two different Message classes), prefer the fully qualified class name (FQCN) such as \UIAwesome\Html\Attribute\Exception\Message::ATTRIBUTE_INVALID_VALUE instead of creating an alias like "use UIAwesome\Html\Attribute\Exception\Message as AttributeMessage". This should apply to PHP files across the codebase (not just tests).
Applied to files:
src/Form/Button.phpsrc/Form/Form.phptests/Form/FormTest.phpsrc/Form/TextArea.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-22T19:37:55.152Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:37:55.152Z
Learning: In PHP test suites for the ui-awesome/html repository, add explicit tests in each element's test file to validate InvalidArgumentException for trait-based attributes (e.g., HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.). Even if these traits are covered in the html-attribute package, ensure coverage is visible in the element tests because test knowledge may be lost when relying solely on trait package tests. Treat this as a guideline that should be applied broadly across element tests to confirm correct behavior and improve maintainability.
Applied to files:
tests/Form/FormTest.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-22T19:29:56.689Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/List/DdTest.php:751-847
Timestamp: 2026-02-22T19:29:56.689Z
Learning: In the ui-awesome/html repository, tests for InvalidArgumentException validation of trait-based attributes (from html-attribute package) should be included in element test files even though the underlying trait behavior is tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite for AI analysis and developer reference.
Applied to files:
tests/Form/FormTest.phpCHANGELOG.mdsrc/Form/TextArea.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-22T20:57:07.467Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 59
File: tests/Form/FormTest.php:980-1132
Timestamp: 2026-02-22T20:57:07.467Z
Learning: In the ui-awesome/html repository, the `HasAutocomplete` trait (from the html-attribute package) does not validate input values and accepts any string without throwing InvalidArgumentException. Therefore, no `testThrowInvalidArgumentExceptionWhenSettingAutocomplete` test should be added. Only traits that use `Validator::oneOf()` (like HasEnctype, HasMethod, etc.) require InvalidArgumentException tests.
Applied to files:
tests/Form/FormTest.phpCHANGELOG.md
📚 Learning: 2026-02-06T21:37:44.509Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 24
File: tests/Form/InputTextTest.php:33-620
Timestamp: 2026-02-06T21:37:44.509Z
Learning: In the ui-awesome/html repository, prefer individual test methods over PHPUnit data providers in test classes. This helps maintainers see exactly how each test behaves without cross-referencing data providers. Apply to all PHP test files under tests. Use separate, descriptively named test methods to cover different inputs/edge cases; retain data providers only if they clearly improve readability or reduce duplication.
Applied to files:
tests/Form/FormTest.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-11T14:56:18.277Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 37
File: tests/Form/InputTextTest.php:448-457
Timestamp: 2026-02-11T14:56:18.277Z
Learning: In PHP test files under the repository ui-awesome/html (e.g., tests/Form/InputTextTest.php), it is acceptable to use short variable names (e.g., 2-character names like $id). PHPMD's short-variable rule does not need to be strictly followed in test code. During reviews, allow short names in test files and note that this relaxed style applies to tests, while production code should adhere to the standard variable naming conventions.
Applied to files:
tests/Form/FormTest.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-21T01:17:40.877Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 51
File: tests/List/UlTest.php:282-283
Timestamp: 2026-02-21T01:17:40.877Z
Learning: In the ui-awesome/html repository, for PHP tests, ensure that assertion messages in tests/List/UlTest.php for methods named testRenderWith*UsingEnum remain identical to the corresponding non-enum tests (i.e., do not append 'using enum' to the failure message). The reason: the underlying render method treats enum and string inputs the same, so failure output should be uniform. Apply this pattern to all similar test cases under tests/ to maintain consistent failure messages across enum vs non-enum variants.
Applied to files:
tests/Form/FormTest.phptests/Form/TextAreaTest.php
📚 Learning: 2026-02-22T19:38:01.916Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:38:01.916Z
Learning: In the ui-awesome/html repository, the maintainer prefers to include InvalidArgumentException validation tests for trait-based attributes (HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.) in each element's test file, even though these traits are tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite because "test knowledge is lost" when relying solely on trait package tests—this explicit coverage is considered "a skill for AI" analysis and developer reference.
Applied to files:
CHANGELOG.mdsrc/Form/TextArea.php
📚 Learning: 2026-02-06T21:37:51.711Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 24
File: tests/Form/InputTextTest.php:33-620
Timestamp: 2026-02-06T21:37:51.711Z
Learning: In the ui-awesome/html repository, prefer individual test methods over PHPUnit data providers in test classes. The maintainer values the ability to see exactly how each test works without needing to cross-reference data providers, even if it results in more methods.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-02-09T11:28:09.334Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 35
File: tests/Provider/Form/CheckedProvider.php:13-14
Timestamp: 2026-02-09T11:28:09.334Z
Learning: In the ui-awesome/html repository, using `str_replace` to adapt shared data provider HTML strings for different input types (e.g., adapting `CheckedProvider` for both `InputCheckboxTest` and `InputRadioTest` by replacing "inputcheckbox-" with "inputradio-" and "checkbox" with "radio") is acceptable and considered clear. Don't suggest refactoring such patterns to avoid string replacement.
Applied to files:
CHANGELOG.md
🧬 Code graph analysis (5)
src/Form/Button.php (2)
src/Form/Form.php (2)
ariaDescribedBySuffix(59-65)getAttributes(74-89)src/Form/TextArea.php (2)
ariaDescribedBySuffix(74-80)getAttributes(89-104)
src/Form/Form.php (2)
src/Form/Button.php (2)
ariaDescribedBySuffix(77-83)getAttributes(92-107)src/Form/TextArea.php (2)
ariaDescribedBySuffix(74-80)getAttributes(89-104)
tests/Form/FormTest.php (1)
src/Form/Form.php (2)
Form(32-102)ariaDescribedBySuffix(59-65)
src/Form/TextArea.php (2)
src/Form/Button.php (2)
ariaDescribedBySuffix(77-83)getAttributes(92-107)src/Form/Form.php (2)
ariaDescribedBySuffix(59-65)getAttributes(74-89)
tests/Form/TextAreaTest.php (3)
src/Form/TextArea.php (2)
TextArea(42-117)ariaDescribedBySuffix(74-80)src/Form/Button.php (1)
ariaDescribedBySuffix(77-83)src/Form/Form.php (1)
ariaDescribedBySuffix(59-65)
🪛 PHPMD (2.15.0)
src/Form/Button.php
[warning] 68-68: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 97-97: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 100-100: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
src/Form/Form.php
[warning] 50-50: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 79-79: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 82-82: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
src/Form/TextArea.php
[warning] 65-65: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
[warning] 94-94: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 97-97: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
🔇 Additional comments (1)
CHANGELOG.md (1)
48-48: Changelog entry for Enh#60is clear and aligned with the implementation.No issues found in this update.
| /** @phpstan-var string|null $id */ | ||
| $id = $this->getAttribute('id', null); | ||
| $ariaDescribedBy = $this->getAttribute('aria-describedby', null); | ||
|
|
||
| $ariaDescribedBySuffix = $this->ariaDescribedBySuffix === '' ? '-help' : "-{$this->ariaDescribedBySuffix}"; | ||
|
|
||
| if ($ariaDescribedBy === true || $ariaDescribedBy === 'true') { | ||
| $attributes['aria-describedby'] = $id !== null ? "{$id}{$ariaDescribedBySuffix}" : null; |
There was a problem hiding this comment.
Normalize empty/whitespace id before composing aria-describedby.
If id is '' (or whitespace), the current branch generates values like -help, which do not reference a valid element.
🛠️ Suggested fix
/** `@phpstan-var` string|null $id */
$id = $this->getAttribute('id', null);
+$id = is_string($id) && trim($id) === '' ? null : $id;
$ariaDescribedBy = $this->getAttribute('aria-describedby', null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** @phpstan-var string|null $id */ | |
| $id = $this->getAttribute('id', null); | |
| $ariaDescribedBy = $this->getAttribute('aria-describedby', null); | |
| $ariaDescribedBySuffix = $this->ariaDescribedBySuffix === '' ? '-help' : "-{$this->ariaDescribedBySuffix}"; | |
| if ($ariaDescribedBy === true || $ariaDescribedBy === 'true') { | |
| $attributes['aria-describedby'] = $id !== null ? "{$id}{$ariaDescribedBySuffix}" : null; | |
| /** `@phpstan-var` string|null $id */ | |
| $id = $this->getAttribute('id', null); | |
| $id = is_string($id) && trim($id) === '' ? null : $id; | |
| $ariaDescribedBy = $this->getAttribute('aria-describedby', null); | |
| $ariaDescribedBySuffix = $this->ariaDescribedBySuffix === '' ? '-help' : "-{$this->ariaDescribedBySuffix}"; | |
| if ($ariaDescribedBy === true || $ariaDescribedBy === 'true') { | |
| $attributes['aria-describedby'] = $id !== null ? "{$id}{$ariaDescribedBySuffix}" : null; |
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 97-97: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 100-100: Avoid excessively long variable names like $ariaDescribedBySuffix. Keep variable name length under 20. (undefined)
(LongVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Form/Button.php` around lines 96 - 103, The code composes
aria-describedby using $id without normalizing empty/whitespace IDs, which
produces invalid values like "-help"; update the logic around getAttribute('id',
null) so $id is normalized (trimmed) and treated as null when it is an empty
string or only whitespace before building $ariaDescribedBySuffix and before
setting $attributes['aria-describedby'] in the branch that checks
$ariaDescribedBy; adjust the condition that assigns
"{$id}{$ariaDescribedBySuffix}" to use the normalized $id (or yield null) and
keep references to getAttribute, $id, $ariaDescribedBySuffix and the
aria-describedby assignment to locate and change the code.
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the HasAttributes.php file from the html-mixin repository to understand the context
curl -s "https://raw.githubusercontent.com/ui-awesome/html-mixin/1c65697ab0bbd6e4e1aa153abd251d8c50ff521b/src/HasAttributes.php" | head -n 100
Length of output: 2868
🏁 Script executed:
#!/bin/bash
# Fetch the AttributeBag class to understand how get() handles empty strings
curl -s "https://raw.githubusercontent.com/ui-awesome/html/main/src/Helper/AttributeBag.php" | grep -A 30 "public static function get"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find AttributeBag in the current repository
fd -t f "AttributeBag.php" --exec cat {}
Length of output: 0
@terabytesoftw, I see you're referencing the getAttribute method implementation. You're right that getAttribute('id', null) delegates to AttributeBag::get() and will return the actual value stored in the attributes array.
The edge case CodeRabbit highlighted is when id is explicitly set to an empty string '' (rather than null or unset). In that scenario:
$id = $this->getAttribute('id', null); // Returns ''
// ...
if ($ariaDescribedBy === true || $ariaDescribedBy === 'true') {
$attributes['aria-describedby'] = $id !== null ? "{$id}{$ariaDescribedBySuffix}" : null;
// Produces: '' . '-help' = '-help' (invalid reference)
}Is this edge case already prevented elsewhere (e.g., does setAttribute('id', '') normalize to null), or would you prefer to add the normalization check as CodeRabbit suggested?
🧠 Learnings used
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:38:01.916Z
Learning: In the ui-awesome/html repository, the maintainer prefers to include InvalidArgumentException validation tests for trait-based attributes (HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.) in each element's test file, even though these traits are tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite because "test knowledge is lost" when relying solely on trait package tests—this explicit coverage is considered "a skill for AI" analysis and developer reference.
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/List/DdTest.php:751-847
Timestamp: 2026-02-22T19:29:56.689Z
Learning: In the ui-awesome/html repository, tests for InvalidArgumentException validation of trait-based attributes (from html-attribute package) should be included in element test files even though the underlying trait behavior is tested in the html-attribute package. The maintainer values having this test knowledge explicitly visible in each element's test suite for AI analysis and developer reference.
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 59
File: tests/Form/FormTest.php:980-1132
Timestamp: 2026-02-22T20:57:07.467Z
Learning: In the ui-awesome/html repository, the `HasAutocomplete` trait (from the html-attribute package) does not validate input values and accepts any string without throwing InvalidArgumentException. Therefore, no `testThrowInvalidArgumentExceptionWhenSettingAutocomplete` test should be added. Only traits that use `Validator::oneOf()` (like HasEnctype, HasMethod, etc.) require InvalidArgumentException tests.
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 31
File: src/Form/Mixin/HasLabel.php:195-195
Timestamp: 2026-02-07T18:51:41.006Z
Learning: In the ui-awesome/html repository, explicit boolean comparisons (e.g., `isset($var) === false`) are required instead of negation operators (e.g., `!isset($var)`) due to PHPStan strict rules configuration at the highest level.
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Sectioning/AsideTest.php:821-833
Timestamp: 2026-02-22T19:26:33.769Z
Learning: In the ui-awesome/html repository, do not use import aliases in PHP. When there are naming conflicts (e.g., two different Message classes), prefer the fully qualified class name (FQCN) such as \UIAwesome\Html\Attribute\Exception\Message::ATTRIBUTE_INVALID_VALUE instead of creating an alias like "use UIAwesome\Html\Attribute\Exception\Message as AttributeMessage". This should apply to PHP files across the codebase (not just tests).
1 participant
Pull Request