feat(attribute): Move attribute traits from ui-awesome/html-core package and update related imports accordingly.
#18
Conversation
…ckage and update related imports accordingly.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 10 seconds before requesting another review. ⌛ 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 (1)
📝 WalkthroughWalkthroughMoves element-specific attribute traits into an Element namespace, replaces literal attribute keys with new enums (Attribute, ElementAttribute, GlobalAttribute, etc.), adds many global attribute traits and enums (including Message), relocates/updates tests and providers, and expands test data/provider shapes. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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. 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: 2
🤖 Fix all issues with AI agents
In @src/Values/ElementAttribute.php:
- Line 28: Fix the typo in the docblock @link for ElementAttribute.php by
removing the extraneous trailing asterisk so the URL reads exactly
"https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/href"; update the
@link line in the ElementAttribute class/file to use the corrected URL.
🧹 Nitpick comments (12)
src/Values/HeaderAttribute.php (1)
7-7: Empty enum placeholder.This enum currently has no cases defined. If this is a placeholder for future header attribute constants, consider adding a comment to clarify the intent, or defer adding this file until cases are ready.
tests/Support/Provider/Global/HiddenProvider.php (1)
87-115: Consider refining the PHPDoc type annotation.The PHPDoc declares the third element as
bool|string, but all test cases use onlybool. Consider updating toarray<string, array{bool, mixed[], bool, string}>for precision, unlessstringis intentionally kept for future extensibility.tests/Support/Provider/Global/DirProvider.php (1)
48-92: Approve with optional type annotation refinement.The implementation correctly uses
EnumDataGeneratorto combine static and enum-based test cases. The array spread operator properly merges both case sets.Consider verifying the PHPDoc type annotation at line 46: if
EnumDataGenerator.cases()always returns strings for the third element (rendered output), the type could be refined fromstring|UnitEnumto juststringfor precision.tests/Global/HasDraggableTest.php (1)
38-39: Inconsistent group annotation.The
#[Group('attributes')]annotation differs from other global attribute tests in this PR which use#[Group('global')](e.g.,CanBeHiddenTest,HasContentEditableTest). Consider using#[Group('global')]for consistency, which would allow running all global attribute tests together.Suggested fix
-#[Group('attributes')] +#[Group('global')]tests/Support/Provider/Global/ContentEditableProvider.php (1)
92-109: Duplicate test cases detected.The
'string'case (lines 92-97) and'string boolean true'case (lines 104-109) both use'true'as input and produce identical expected output. Consider removing the redundant'string'case or updating it to test a different valid value like'plaintext-only':♻️ Suggested fix
'string' => [ - 'true', + 'plaintext-only', [], - ' contenteditable="true"', + ' contenteditable="plaintext-only"', 'Should return the attribute value after setting it.', ],The same applies to the
values()method at lines 169-186.src/Global/HasContentEditable.php (1)
69-78: Add explicit null/empty string handling for consistency and clarity.The test suite confirms that
nulland empty string are expected to work correctly (unset/empty rendering), as shown inContentEditableProvider. However, the implementation does not explicitly handle these cases before validation—instead relying onValidator::oneOfto handle them.For consistency with the pattern used in
HasClassand to be more explicit about edge case handling, add a guard clause:Suggested change
public function contentEditable(bool|string|UnitEnum|null $value): static { if (is_bool($value)) { $value = $value ? 'true' : 'false'; } + if ($value === '' || $value === null) { + return $this->addAttribute(GlobalAttribute::CONTENTEDITABLE, $value); + } + Validator::oneOf($value, ContentEditable::cases(), GlobalAttribute::CONTENTEDITABLE); return $this->addAttribute(GlobalAttribute::CONTENTEDITABLE, $value); }This matches the approach in
HasClass, ensures null/empty string bypass validation, and makes the intent explicit.tests/Support/Provider/Global/AriaProvider.php (1)
48-90: Consider extracting shared invalid key test cases to reduce duplication.The
invalidKey()andinvalidSingleKey()methods are exact duplicates ofDataProvider::invalidKey()andDataProvider::invalidSingleKey()(as shown in the relevant code snippets at lines 47-63 and 77-89 ofDataProvider.php). This violates DRY.Consider either:
- Calling
DataProvider::invalidKey()/DataProvider::invalidSingleKey()directly in tests- Extracting these to a shared base provider class
tests/Support/Provider/Global/DraggableProvider.php (2)
91-108: Duplicate test cases detected.The
'string'case (lines 91-96) and'string boolean true'case (lines 103-108) are identical—both use input'true'and expect' draggable="true"'. One of these should be removed.♻️ Suggested removal
'string' => [ 'true', [], ' draggable="true"', 'Should return the attribute value after setting it.', ], - 'string boolean false' => [ - 'false', - [], - ' draggable="false"', - 'Should return the attribute value after setting it.', - ], - 'string boolean true' => [ - 'true', - [], - ' draggable="true"', - 'Should return the attribute value after setting it.', - ], + 'string boolean false' => [ + 'false', + [], + ' draggable="false"', + 'Should return the attribute value after setting it.', + ],
168-185: Same duplicate pattern invalues()method.The
'string'case (lines 168-173) and'string boolean true'case (lines 180-185) are identical duplicates, mirroring the issue inrenderAttribute().tests/Global/HasDirTest.php (1)
108-113: Inconsistent attribute key access.Line 110 uses the string literal
'dir'to access the attribute, but line 126-127 usesGlobalAttribute::DIR->valuefor the exception message. For consistency, consider using the enum value here as well:♻️ Suggested fix
self::assertSame( $expected, - $instance->getAttributes()['dir'] ?? '', + $instance->getAttributes()[GlobalAttribute::DIR->value] ?? '', $message, );tests/Element/HasReferrerpolicyTest.php (1)
108-113: Inconsistent attribute key access.Line 110 uses the string literal
'referrerpolicy'to access the attribute, while line 126 usesElementAttribute::REFERRERPOLICY->valuefor the exception message. For consistency, use the enum value:♻️ Suggested fix
self::assertSame( $expected, - $instance->getAttributes()['referrerpolicy'] ?? '', + $instance->getAttributes()[ElementAttribute::REFERRERPOLICY->value] ?? '', $message, );src/Values/Aria.php (1)
144-149: Consider noting deprecated status forDROPEFFECTandGRABBED.Both
aria-dropeffectandaria-grabbedare deprecated in ARIA 1.1. Consider adding a@deprecatedannotation or noting this in the docblock to inform consumers about their status.📝 Example documentation update
/** * `aria-dropeffect` — Indicates what functions may be performed when a dragged object is released. * + * @deprecated Deprecated in ARIA 1.1 in favor of using the HTML Drag and Drop API. * @link https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-dropeffect */ case DROPEFFECT = 'dropeffect';/** * `aria-grabbed` — Indicates an element's "grabbed" state in a drag-and-drop operation. * + * @deprecated Deprecated in ARIA 1.1 in favor of using the HTML Drag and Drop API. * @link https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-grabbed */ case GRABBED = 'grabbed';Also applies to: 172-177
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
CHANGELOG.mdsrc/Element/HasAlt.phpsrc/Element/HasHeight.phpsrc/Element/HasHref.phpsrc/Element/HasReferrerpolicy.phpsrc/Element/HasSrc.phpsrc/Element/HasWidth.phpsrc/Exception/Message.phpsrc/Global/CanBeAutofocus.phpsrc/Global/CanBeHidden.phpsrc/Global/HasAccesskey.phpsrc/Global/HasAria.phpsrc/Global/HasClass.phpsrc/Global/HasContentEditable.phpsrc/Global/HasData.phpsrc/Global/HasDir.phpsrc/Global/HasDraggable.phpsrc/HasCrossorigin.phpsrc/HasRel.phpsrc/Values/Aria.phpsrc/Values/Attribute.phpsrc/Values/ContentEditable.phpsrc/Values/Data.phpsrc/Values/Direction.phpsrc/Values/Draggable.phpsrc/Values/ElementAttribute.phpsrc/Values/GlobalAttribute.phpsrc/Values/HeaderAttribute.phptests/Element/HasAltTest.phptests/Element/HasHeightTest.phptests/Element/HasHrefTest.phptests/Element/HasReferrerpolicyTest.phptests/Element/HasSrcTest.phptests/Element/HasWidthTest.phptests/Global/CanBeAutofocusTest.phptests/Global/CanBeHiddenTest.phptests/Global/HasAccesskeyTest.phptests/Global/HasAriaTest.phptests/Global/HasClassTest.phptests/Global/HasContentEditableTest.phptests/Global/HasDataTest.phptests/Global/HasDirTest.phptests/Global/HasDraggableTest.phptests/HasCrossoriginTest.phptests/HasRelTest.phptests/Support/Provider/CrossoriginProvider.phptests/Support/Provider/Element/AltProvider.phptests/Support/Provider/Element/HeightProvider.phptests/Support/Provider/Element/HrefProvider.phptests/Support/Provider/Element/ReferrerpolicyProvider.phptests/Support/Provider/Element/SrcProvider.phptests/Support/Provider/Element/WidthProvider.phptests/Support/Provider/Global/AccesskeyProvider.phptests/Support/Provider/Global/AriaProvider.phptests/Support/Provider/Global/AutofocusProvider.phptests/Support/Provider/Global/ClassProvider.phptests/Support/Provider/Global/ContentEditableProvider.phptests/Support/Provider/Global/DataProvider.phptests/Support/Provider/Global/DirProvider.phptests/Support/Provider/Global/DraggableProvider.phptests/Support/Provider/Global/HiddenProvider.phptests/Support/Provider/RelProvider.phptests/Support/Stub/Values/AlertType.phptests/Support/Stub/Values/ButtonSize.phptests/Support/Stub/Values/Priority.php
🧰 Additional context used
🧬 Code graph analysis (25)
tests/Support/Provider/Global/HiddenProvider.php (1)
tests/Support/Provider/Global/AutofocusProvider.php (2)
renderAttribute(44-72)values(87-115)
tests/Support/Provider/Global/AutofocusProvider.php (1)
tests/Support/Provider/Global/HiddenProvider.php (2)
renderAttribute(44-72)values(87-115)
tests/Support/Provider/Global/AccesskeyProvider.php (7)
tests/Support/Provider/Element/WidthProvider.php (2)
renderAttribute(42-76)values(91-125)tests/Support/Provider/Global/AriaProvider.php (2)
renderAttribute(105-245)values(423-557)tests/Support/Provider/Global/AutofocusProvider.php (2)
renderAttribute(44-72)values(87-115)tests/Support/Provider/Global/ClassProvider.php (2)
renderAttribute(49-121)values(138-215)tests/Support/Provider/Global/ContentEditableProvider.php (2)
renderAttribute(50-119)values(134-196)tests/Support/Provider/Global/DirProvider.php (2)
renderAttribute(48-92)values(107-145)tests/Support/Provider/Global/HiddenProvider.php (2)
renderAttribute(44-72)values(87-115)
src/Global/HasDir.php (3)
tests/Support/Provider/Global/AriaProvider.php (1)
value(264-407)tests/Support/Provider/Global/DataProvider.php (1)
value(264-401)tests/Support/EnumDataGenerator.php (1)
cases(42-64)
tests/Global/HasAccesskeyTest.php (2)
tests/Support/Provider/Global/AccesskeyProvider.php (1)
AccesskeyProvider(29-128)src/Global/HasAccesskey.php (1)
accesskey(54-57)
tests/Element/HasHrefTest.php (1)
tests/Support/Provider/Element/HrefProvider.php (1)
HrefProvider(29-152)
src/Global/HasClass.php (2)
tests/Support/Provider/Global/AriaProvider.php (1)
value(264-407)tests/Support/Provider/Global/DataProvider.php (1)
value(264-401)
tests/Global/HasDraggableTest.php (3)
tests/Support/Provider/Global/DraggableProvider.php (1)
DraggableProvider(35-196)src/Global/HasDraggable.php (1)
draggable(69-78)src/Exception/Message.php (1)
getMessage(63-66)
tests/Support/Provider/CrossoriginProvider.php (1)
tests/Support/EnumDataGenerator.php (1)
EnumDataGenerator(25-89)
tests/Support/Provider/Global/AriaProvider.php (2)
tests/Support/Provider/Global/DataProvider.php (3)
invalidKey(48-64)invalidSingleKey(78-90)value(264-401)tests/Support/EnumDataGenerator.php (1)
cases(42-64)
tests/Global/CanBeHiddenTest.php (2)
tests/Support/Provider/Global/HiddenProvider.php (1)
HiddenProvider(29-116)src/Global/CanBeHidden.php (1)
hidden(54-57)
src/Global/HasData.php (1)
src/Exception/Message.php (1)
getMessage(63-66)
tests/Support/Provider/Global/DraggableProvider.php (1)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(25-89)cases(42-64)
src/Global/HasAria.php (1)
src/Exception/Message.php (1)
getMessage(63-66)
tests/Element/HasSrcTest.php (1)
tests/Support/Provider/Element/SrcProvider.php (1)
SrcProvider(27-126)
tests/Element/HasWidthTest.php (1)
tests/Support/Provider/Element/WidthProvider.php (1)
WidthProvider(27-126)
tests/Global/HasDirTest.php (3)
tests/Support/Provider/Global/DirProvider.php (1)
DirProvider(33-146)src/Global/HasDir.php (1)
dir(67-72)src/Exception/Message.php (1)
getMessage(63-66)
tests/Global/HasAriaTest.php (3)
tests/Support/Provider/Global/AriaProvider.php (2)
AriaProvider(34-558)value(264-407)src/Global/HasAria.php (3)
ariaAttributes(133-150)addAriaAttribute(85-94)removeAriaAttribute(170-175)src/Exception/Message.php (1)
getMessage(63-66)
tests/Global/HasContentEditableTest.php (3)
tests/Support/Provider/Global/ContentEditableProvider.php (1)
ContentEditableProvider(35-197)src/Global/HasContentEditable.php (1)
contentEditable(69-78)src/Exception/Message.php (1)
getMessage(63-66)
tests/Global/HasDataTest.php (4)
tests/Support/Provider/Global/DataProvider.php (2)
DataProvider(34-552)value(264-401)src/Global/HasData.php (3)
dataAttributes(124-141)addDataAttribute(77-86)removeDataAttribute(161-166)tests/Support/Provider/Global/AriaProvider.php (1)
value(264-407)src/Exception/Message.php (1)
getMessage(63-66)
tests/Element/HasHeightTest.php (1)
tests/Support/Provider/Element/HeightProvider.php (1)
HeightProvider(28-127)
tests/Global/CanBeAutofocusTest.php (2)
tests/Support/Provider/Global/AutofocusProvider.php (1)
AutofocusProvider(29-116)src/Global/CanBeAutofocus.php (1)
autofocus(54-57)
tests/Support/Provider/Global/ClassProvider.php (4)
tests/Support/Provider/Element/HrefProvider.php (2)
renderAttribute(44-90)values(105-151)tests/Support/Provider/Global/AccesskeyProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/AriaProvider.php (2)
renderAttribute(105-245)values(423-557)tests/Support/Provider/Global/DirProvider.php (2)
renderAttribute(48-92)values(107-145)
tests/HasRelTest.php (1)
tests/Support/Provider/RelProvider.php (1)
RelProvider(31-144)
tests/Element/HasReferrerpolicyTest.php (1)
tests/Support/Provider/Element/ReferrerpolicyProvider.php (1)
ReferrerpolicyProvider(34-148)
⏰ 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). (8)
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (92)
tests/Support/Stub/Values/AlertType.php (1)
1-37: LGTM! Well-structured test support enum.The
AlertTypeenum is correctly implemented with appropriate string-backed values and comprehensive documentation. It aligns well with the PR's objective of introducing strongly-typed attribute identifiers for the test suite.tests/Support/Stub/Values/Priority.php (1)
1-26: LGTM! Clean test support enum.The enum is well-structured with comprehensive documentation. The priority values (HIGH=2, LOW=1) provide a logical ordering for test scenarios involving sorting and prioritization logic.
src/Values/Direction.php (1)
1-48: LGTM!The Direction enum is well-documented with accurate values matching the HTML specification. The comprehensive documentation explains each case clearly with appropriate examples.
src/Values/Attribute.php (1)
1-185: LGTM!The Attribute enum provides a comprehensive, type-safe collection of HTML attribute constants. Documentation is thorough with MDN references for each case, and all attribute values correctly match HTML specifications.
src/Exception/Message.php (1)
1-67: LGTM!The Message enum provides a clean, centralized approach to error message handling with formatted templates. The
getMessage()method correctly usessprintffor dynamic interpolation, and the documentation includes helpful usage examples.tests/Support/Provider/Global/HiddenProvider.php (1)
44-72: LGTM! Well-structured test cases for boolean attribute rendering.The test data comprehensively covers boolean scenarios (true/false) and attribute replacement behavior, with clear assertion messages for debugging.
tests/Support/Provider/Global/AutofocusProvider.php (1)
44-115: LGTM! Consistent implementation following established patterns.The data provider follows the same well-structured pattern as
HiddenProvider, with comprehensive test coverage for the booleanautofocusattribute. The same optional PHPDoc type refinement mentioned forHiddenProvider.values()applies here (line 85: considerboolinstead ofbool|stringfor the third element).tests/Support/Provider/Global/DirProvider.php (2)
1-33: LGTM! Clean imports and comprehensive documentation.The file properly imports necessary dependencies and provides thorough documentation explaining the test data's purpose and coverage.
107-145: LGTM! Proper implementation of value test cases.The method correctly combines static and enum-based test cases. The
falseparameter passed toEnumDataGenerator.cases()(line 109) likely controls whether values are returned as strings or enum instances, justifying thestring|UnitEnumtype annotation.src/Values/GlobalAttribute.php (2)
1-24: LGTM! Excellent documentation and proper enum structure.The file provides comprehensive documentation with clear design goals and external references. The string-backed enum is the appropriate choice for representing HTML attribute names.
26-263: Comprehensive and well-documented attribute enum.The enum provides excellent coverage of HTML global attributes with proper:
- Individual documentation for each case with MDN links
- Consistent naming convention (note:
CLASS_CSSappropriately avoids PHP keyword conflict)- Lowercase string values matching HTML specification
- No apparent omissions or duplicates
tests/Support/Stub/Values/ButtonSize.php (1)
1-32: LGTM! Clean and simple test stub enum.The enum provides a clear, type-safe set of button size values for testing scenarios. The implementation follows best practices with proper documentation and conventional size abbreviations.
CHANGELOG.md (1)
9-9: LGTM!The changelog entry accurately documents the enhancement and follows the established format.
tests/Support/Provider/Element/AltProvider.php (1)
5-8: LGTM!The namespace migration from Media to Element is consistent with the broader refactoring to relocate attribute traits. The docblock reference has been correctly updated to reflect the new test location.
tests/Support/Provider/CrossoriginProvider.php (1)
5-12: LGTM!The namespace migration is consistent with the trait relocation. The placement in the root
Providernamespace (rather thanProvider\Element) aligns withHasCrossoriginbeing a general-purpose attribute used across multiple element types.tests/Element/HasHrefTest.php (2)
5-10: LGTM!The test namespace and imports have been correctly updated to reflect the trait relocation from Link to Element namespace. The migration is consistent with the broader refactoring effort.
33-33: LGTM!The PHPUnit group annotation has been appropriately updated from 'link' to 'element' to reflect the new categorization.
src/HasCrossorigin.php (2)
8-8: LGTM!The import has been updated to include the
Attributeenum, which provides centralized attribute name constants. This supports the enum-based approach on lines 65 and 67.
65-67: Excellent improvement: Using enum constants instead of string literals.Replacing string literals with
Attribute::CROSSORIGINprovides better type safety, IDE autocomplete support, and refactoring capabilities. TheAttributeenum is properly defined with theCROSSORIGINconstant.src/Element/HasHeight.php (3)
5-7: LGTM: Namespace refactoring is clean.The namespace change from
MediatoElementand the addition of theElementAttributeimport are consistent with the PR's objective to centralize attribute trait definitions.
24-26: LGTM: Documentation references are comprehensive.The updated documentation links provide clear references to MDN, WHATWG HTML, and CSS specifications for the
heightattribute across different contexts (HTML, SVG, CSS).Also applies to: 48-49
63-66: LGTM: Enum-based attribute key improves type safety.The refactored implementation using
ElementAttribute::HEIGHTinstead of a string literal provides better type safety and IDE support while maintaining the same public API. The enum is properly defined with the HEIGHT case, and the method correctly maintains the immutable pattern with thestaticreturn type.tests/Support/Provider/Element/HrefProvider.php (1)
5-8: LGTM: Test provider namespace aligned with refactoring.The namespace and documentation updates correctly reflect the movement of
HasHrefTestfrom theLinknamespace to theElementnamespace. No functional changes to test data.tests/Support/Provider/Element/ReferrerpolicyProvider.php (1)
5-12: LGTM: Test provider correctly relocated to Element namespace.The namespace change and docblock reference update align with the broader refactoring to move
HasReferrerpolicyTestfromLinktoElementnamespace. The use ofReferrerpolicyenum ensures type-safe test data generation.tests/Support/Provider/Element/SrcProvider.php (1)
5-8: LGTM: Test provider namespace refactored correctly.The namespace and docblock updates are consistent with the relocation of
HasSrcTestfrom theMedianamespace to theElementnamespace. Test data remains functionally unchanged.src/Element/HasWidth.php (3)
5-7: LGTM: Namespace refactoring is consistent.The namespace change to
Elementand the addition of theElementAttributeimport mirror the refactoring applied to other attribute traits in this PR.
24-26: LGTM: Documentation is thorough and well-referenced.The updated documentation links provide proper references to MDN, WHATWG HTML, and CSS specifications for the
widthattribute, covering HTML, SVG, and CSS contexts.Also applies to: 48-49
66-69: LGTM: Enum-based attribute key enhances type safety.The implementation using
ElementAttribute::WIDTHimproves type safety and prevents string literal typos while preserving the public API contract. TheWIDTHconstant is properly defined in the enum with appropriate documentation.tests/Support/Provider/RelProvider.php (1)
5-5: LGTM! Clean namespace reorganization.The namespace and documentation updates correctly reflect the relocation of the
HasReltrait from theLinkcontext to the broaderAttributecontext, aligning with the PR's objective to consolidate attribute traits.Also applies to: 12-12
src/Global/CanBeHidden.php (1)
31-58: LGTM! Well-structured global attribute trait.The trait follows the established immutable API pattern seen throughout the codebase, with clear documentation, proper type safety, and appropriate delegation to
addAttribute(). The use of theGlobalAttribute::HIDDENenum constant ensures consistency with the broader attribute key standardization effort.src/Values/ContentEditable.php (1)
24-46: LGTM! Standards-compliant enum implementation.The enum correctly represents the three valid values for the HTML
contenteditableattribute ('false','plaintext-only','true') as specified in the HTML standard. The use of a backed string enum provides type safety while maintaining semantic clarity.tests/Support/Provider/Element/HeightProvider.php (1)
5-5: LGTM! Namespace reorganization correctly applied.The namespace migration from
MediatoElementand the corresponding documentation update align with the PR's broader effort to reorganize attribute traits into more appropriate contexts.Also applies to: 8-8
tests/Global/HasClassTest.php (1)
36-119: LGTM! Comprehensive test coverage with clean implementation.The test suite effectively validates:
- Attribute rendering with various inputs via data providers
- Immutability guarantees of the trait API
- Empty state handling
- Sequential operations and override behavior
The use of anonymous classes for trait composition and match expressions for conditional override handling follows modern PHP best practices.
src/Element/HasAlt.php (1)
5-7: LGTM: Clean refactoring to use enum constants.The migration from Media namespace to Element namespace and the use of
ElementAttribute::ALTinstead of a string literal improves type safety and maintainability. This aligns with the broader PR goal of centralizing attribute keys.Also applies to: 64-64
src/Element/HasHref.php (1)
5-7: LGTM: Consistent refactoring pattern.The namespace migration and use of
ElementAttribute::HREFfollows the same clean refactoring pattern as other Element traits in this PR. The change improves consistency and type safety across the codebase.Also applies to: 68-68
tests/Support/Provider/Element/WidthProvider.php (1)
5-8: LGTM: Namespace alignment for test provider.The namespace change from Media to Element correctly aligns this test provider with the migration of width-related traits to the Element namespace.
src/Global/HasDir.php (1)
67-72: Validator::oneOf correctly handles null values. The test suite inDirProvider.phpexplicitly includes test cases for null handling:'null'and'unset with null'. These tests demonstrate that null is a supported value and should work without throwing an exception, which contradicts the original concern. TheValidator::oneOfmethod from the external dependency must properly handle null inputs.src/Global/HasClass.php (1)
82-86: The implementation correctly uses a consistent attribute key for both operations. Line 83 unsets usingGlobalAttribute::CLASS_CSS->value(which is'class'), and the test data showsCSSClass::add()successfully appends to and manages the same'class'attribute. The append test case on line 54 of ClassProvider demonstrates that when an existing'class'attribute is present,CSSClass::add()correctly appends to it, which could only occur if both operations use the same key. No key inconsistency issue exists.src/Values/Draggable.php (1)
24-39: LGTM! Well-structured enum for HTML draggable attribute.The enum correctly represents the two valid string values ('false' and 'true') for the HTML draggable attribute according to the specification. The documentation is comprehensive and the implementation follows best practices for type-safe attribute value handling.
src/Global/HasAccesskey.php (1)
54-57: LGTM! Clean implementation of the accesskey attribute trait.The trait correctly implements the HTML accesskey global attribute with proper immutability, type safety via the GlobalAttribute enum, and support for nullable values. The documentation is thorough and includes practical usage examples.
src/Values/Data.php (1)
24-130: LGTM! Comprehensive enum for data- attribute names.*The enum provides a well-organized, type-safe set of commonly used data-* attribute names. Each case is properly documented with its purpose and typical use case. The selected attribute names cover standard patterns for JavaScript event delegation, UI component behavior (modals, tooltips, toggles), and data storage, aligning well with modern web development practices.
tests/Element/HasAltTest.php (1)
5-10: LGTM! Namespace migration is consistent and complete.The test file has been correctly migrated from the Media namespace to the Element namespace. All related updates are consistent:
- Test class namespace updated
- Trait and provider imports updated to Element namespace
- PHPUnit group annotation changed to 'element'
The test logic remains unchanged, ensuring continuity in test coverage while aligning with the new organizational structure.
Also applies to: 33-33
src/Global/CanBeAutofocus.php (1)
54-57: The implementation is correct and requires no changes.
autofocusis a boolean attribute per the HTML Living Standard, which correctly renders as justautofocuswhen true and is omitted entirely when false—exactly as the test data confirms. This differs fundamentally fromdraggable, which is an enumerated attribute requiring string values ('true'/'false') per the spec. The framework's rendering system (fromui-awesome/html-helper) already handles boolean attributes according to the specification. The current approach of passingbooldirectly toaddAttributeis the correct design for boolean attributes likeautofocusandhidden.Likely an incorrect or invalid review comment.
src/Element/HasSrc.php (2)
5-7: LGTM! Namespace migration and enum import are well-structured.The relocation to the
Elementnamespace and the addition of theElementAttributeenum import align with the PR's objective to centralize attribute management and improve type safety.
61-64: LGTM! Enum-based attribute key improves type safety.The migration from a string literal
'src'toElementAttribute::SRCis a solid improvement that provides compile-time validation and eliminates potential typos.tests/Support/Provider/Global/AccesskeyProvider.php (1)
29-128: LGTM! Test provider follows established patterns.The
AccesskeyProviderclass is well-structured and provides comprehensive test coverage for theaccesskeyattribute, including edge cases like empty strings, null values, and attribute replacement. The pattern of having separaterenderAttribute()andvalues()methods is consistent with other global attribute providers in the codebase.src/Global/HasDraggable.php (1)
71-73: LGTM! Boolean normalization is HTML spec-compliant.The conversion of boolean values to
'true'or'false'strings aligns with the HTML specification for thedraggableattribute.tests/Element/HasSrcTest.php (2)
5-10: LGTM! Test namespace migration is consistent.The namespace and import updates correctly reflect the trait's relocation from
MediatoElementnamespace, maintaining consistency across the test suite.
33-110: LGTM! Test suite maintains comprehensive coverage.The test methods provide thorough coverage for the
HasSrctrait:
- Attribute rendering validation
- Empty state verification
- Immutability checks
- Value assignment tests
The test group annotation correctly reflects the
Elementnamespace.tests/Global/CanBeAutofocusTest.php (1)
1-111: LGTM! Comprehensive test coverage for CanBeAutofocus trait.The test suite properly validates:
- Attribute rendering with various boolean values
- Empty state when attribute is not set
- Immutability of the fluent API
- Correct value storage and retrieval
The use of
DataProviderExternalwithAutofocusProviderand the anonymous class pattern for trait testing are consistent with the established conventions in this codebase.src/HasRel.php (2)
5-9: LGTM! Namespace and import updates align with the centralized enum approach.The namespace change and grouped import of
{Attribute, Rel}properly reflect the restructuring. UsingAttribute::RELas the key ensures type safety and consistency across the codebase.
63-68: LGTM! Method implementation is correct and follows immutable patterns.The validation with
Validator::oneOfprecedes the attribute assignment, ensuring invalid values are rejected before mutation. The use ofAttribute::RELenum constant maintains consistency with other trait implementations in this PR.tests/Global/HasContentEditableTest.php (4)
1-17: LGTM! Test file structure and imports are well-organized.The test class follows established patterns for testing traits with anonymous classes. Imports are appropriate for the test scenarios covered.
38-63: LGTM! Data-driven render test is well-structured.The test method properly validates attribute rendering using the external data provider. The assertion message from the provider aids in debugging failed tests.
65-90: LGTM! Empty state and immutability tests are correct.Both tests properly verify the trait's behavior: no attributes when unset, and new instances returned on mutation (immutability).
116-133: LGTM! Exception test validates input constraints correctly.The test ensures invalid values are rejected with a descriptive exception message, using the
Message::VALUE_NOT_IN_LISTenum for consistent error formatting.tests/Support/Provider/Global/ClassProvider.php (2)
34-121: LGTM! Comprehensive test data for class attribute scenarios.The
renderAttribute()provider covers essential scenarios including append, replace, enum values, and null handling. The override flag parameter enables testing both append and replace behaviors. The inlineStringableimplementation is a valid approach for testing.
138-215: LGTM! Thevalues()provider supports multi-step class attribute operations.The nested array structure with
valueandoverridekeys enables testing sequential class operations (append followed by override, etc.). This design aligns with how theclassattribute accumulates values in real usage.src/Element/HasReferrerpolicy.php (3)
5-10: LGTM! Namespace and imports correctly updated for Element placement.The trait is appropriately placed in the
Elementnamespace sincereferrerpolicyapplies to multiple HTML elements (a, area, iframe, img, link, script). The grouped import maintains consistency with other traits.
27-32: LGTM! Documentation links are comprehensive.The expanded
@linkannotations covering all relevant HTML elements (a, area, iframe, img, link, script) improve discoverability and help developers understand where this trait applies.
68-73: LGTM! Implementation follows established patterns.The method correctly validates against
Referrerpolicy::cases()before setting the attribute using theElementAttribute::REFERRERPOLICYenum constant.tests/Element/HasHeightTest.php (2)
5-13: LGTM! Namespace and imports correctly migrated to Element.The test file properly reflects the trait's move from
MediatoElementnamespace. The#[Group('element')]annotation is consistent with the new location.
33-109: LGTM! Test coverage is appropriate for the height attribute.The tests cover rendering, empty state, immutability, and value assertions. Unlike attributes with enumerated values,
heightaccepts free-form strings, so omitting an invalid-value exception test is correct.tests/Global/CanBeHiddenTest.php (2)
1-17: LGTM! Test file structure is appropriate for boolean-only attribute.The imports and namespace are correctly configured. Since
hidden()accepts onlybool, no invalid argument exception test is needed.
33-109: LGTM! Test coverage appropriately matches the trait's boolean-only API.The four test methods cover rendering, empty state, immutability, and value storage. The
bool|stringexpected type intestSetHiddenAttributeValuecorrectly aligns with the provider's return type.tests/Global/HasDraggableTest.php (1)
40-133: LGTM! Test implementation is thorough and correct.The test methods properly cover all key scenarios: data-driven rendering, empty state, immutability, value assertions, and exception handling for invalid inputs. The exception test correctly validates against the
Draggableenum cases.tests/Global/HasAccesskeyTest.php (1)
1-111: LGTM!The test suite is well-structured with comprehensive coverage for:
- Rendering with data provider scenarios
- Empty state verification
- Immutability checks
- Value assignment validation
The use of anonymous classes and external data providers follows the established patterns in this codebase.
tests/Element/HasWidthTest.php (1)
5-33: LGTM!The namespace migration from
MediatoElementis correctly applied:
- Namespace updated to
UIAwesome\Html\Attribute\Tests\Element- Imports reference
Element\HasWidthandElement\WidthProvider- Group attribute updated to
'element'The test logic remains intact, ensuring consistent behavior after the refactoring.
tests/HasRelTest.php (2)
110-112: Good use of enum constants for attribute key access.Using
Attribute::REL->valueinstead of the hardcoded'rel'string ensures consistency with the trait implementation and makes the test more resilient to potential attribute key changes.
123-129: LGTM!The exception message construction correctly uses
Attribute::REL->valueto dynamically reference the attribute name, maintaining consistency with the enum-based approach throughout the codebase.tests/Support/Provider/Global/ContentEditableProvider.php (1)
35-119: LGTM on overall structure.The provider offers comprehensive coverage for:
- Boolean inputs (true/false)
- String values ('true', 'false', 'plaintext-only')
- Enum values via
EnumDataGenerator- Null handling and attribute unsetting
- Replacement scenarios
The use of
EnumDataGenerator::cases()to dynamically generate enum-based test cases is a clean approach.src/Global/HasAria.php (3)
85-94: LGTM onaddAriaAttributeimplementation.The method correctly:
- Clones the instance for immutability
- Uses the
'aria-'prefix andboolToString = truefor proper ARIA boolean handling- Supports flexible value types including
Closurefor dynamic scenarios
170-175: LGTM onremoveAriaAttributeimplementation.The method properly normalizes the key with the
'aria-'prefix before removal, ensuring consistent handling regardless of whether the user provides the key with or without the prefix.
133-150: Key validation is already handled by thesetAttributemethod.The
ariaAttributesmethod delegates key validation tosetAttribute, which throwsInvalidArgumentExceptionwithMessage::KEY_MUST_BE_NON_EMPTY_STRINGfor invalid keys. The test suite confirms this behavior is working as intended—no additional validation is needed.tests/Global/HasDataTest.php (3)
68-90: Comprehensive immutability testing.Good coverage testing that all three methods (
addDataAttribute,dataAttributes,removeDataAttribute) return new instances, ensuring the trait maintains immutability across all operations.
157-170: LGTM on invalid value exception test.The test correctly verifies that passing an invalid object type (
stdClass) triggers the expectedInvalidArgumentExceptionwith the appropriate error message for non-scalar/non-closure values.
172-188: Address mixed validation scenarios in test data provider.The test data provider
invalidSingleKey()returns two semantically different invalid cases: an empty string and aUnitEnum. While both types match the method signaturestring|UnitEnum $key, combining them in a single test expecting the error messageKEY_MUST_BE_NON_EMPTY_STRINGis problematic. The empty string case correctly validates against this message, but theUnitEnumcase represents a different semantic failure (non-string key when string validation is expected) and shouldn't share the same error message. Either split into separate test methods or verify that both cases actually throw the same exception message and document why.tests/Support/Provider/Global/AriaProvider.php (1)
105-245: LGTM!The
renderAttribute(),value(), andvalues()methods provide comprehensive test coverage for aria-* attributes with good edge case handling (closures, enums, null values, stringable, key normalization). The dynamic enum case generation invalues()is a clean approach.Also applies to: 264-407, 423-557
tests/Support/Provider/Global/DataProvider.php (1)
1-552: LGTM!This provider serves as the canonical implementation for invalid key test cases that are duplicated in
AriaProvider. The boolean handling difference (data-* uses boolean attributes wheretruerenders as presence-onlydata-valueandfalseomits the attribute) correctly follows HTML data attribute semantics, distinct from aria-* which renders explicit"true"/"false"strings.tests/Global/HasDirTest.php (1)
1-133: LGTM overall!The test suite provides good coverage for the
HasDirtrait: rendering, empty state, immutability, value setting, and exception handling. The use of data providers and anonymous class composition follows established patterns in the codebase.tests/HasCrossoriginTest.php (1)
5-16: LGTM!The namespace, imports, and enum usage have been updated consistently. The test correctly uses
Attribute::CROSSORIGIN->valuefor attribute key access (line 111) and exception message construction (line 127), aligning with the codebase's move to centralized enum-based attribute keys.Also applies to: 38-38, 111-111, 127-127
tests/Element/HasReferrerpolicyTest.php (1)
1-133: LGTM overall!The test suite correctly validates the
HasReferrerpolicytrait with proper namespace updates, enum-based validation, and comprehensive test coverage through data providers.src/Global/HasData.php (3)
128-138: Defensive error handling is appropriate.The
@phpstan-ignore catch.neverThrownannotation acknowledges that static analysis can't detect TypeErrors from loosely-typed array values at runtime. The catch block appropriately translates the low-level TypeError into a domain-specificInvalidArgumentExceptionwith a clear message.
1-167: LGTM!The trait provides a clean, immutable API for managing
data-*attributes with:
- Proper key normalization with
data-prefix- Support for scalar, Closure, Stringable, and UnitEnum values
- Comprehensive PHPDoc with usage examples
- Defensive error handling for bulk operations
161-166: No changes needed—immutability pattern is correctly implemented.The
removeDataAttributemethod correctly maintains immutability. The PHPDoc in HasAria.php explicitly documents that@method static removeAttribute(string $key) Removes an attribute and returns a new instance., and the method documentation forremoveDataAttributeconfirms it "Creates a new instance with the specified custom data attribute removed."Unlike
addDataAttributeanddataAttributeswhich explicitly clone before callingsetAttribute, theremoveDataAttributemethod delegates immutability handling to theremoveAttributemethod itself, which returns a new instance directly. Both approaches correctly maintain the immutable contract—the difference is where the cloning occurs. The identical pattern used in bothremoveDataAttributeandremoveAriaAttributeconfirms this is the intended design.Likely an incorrect or invalid review comment.
src/Values/Aria.php (1)
1-23: Well-structured ARIA enum with comprehensive documentation.The enum provides a type-safe, standards-compliant set of ARIA attribute identifiers with excellent MDN documentation links. The namespace organization under
Valuesis clean.tests/Global/HasAriaTest.php (6)
1-19: Clean test file structure with appropriate imports.All imports are properly used throughout the test class. The namespace and use of PHPUnit 10+ attributes (
#[DataProviderExternal],#[Group]) follows modern PHPUnit conventions.
43-66: LGTM!Test method properly validates aria attribute rendering using external data provider. The assertion message parameter aids debugging when tests fail.
68-79: LGTM!Clean baseline test verifying no attributes are present on a fresh instance.
105-149: LGTM!Both attribute value tests correctly validate storage behavior using external data providers. The PHPStan annotations provide proper type coverage for the varied value types.
151-201: LGTM!Exception tests properly validate error handling for invalid keys and values. The use of
stdClassto trigger the invalid value exception is appropriate, and the Message enum integration correctly formats the expected exception messages.
81-103: The test is correctly written and will pass.The
removeAriaAttributemethod's docstring explicitly states it "returns a new instance with the specifiedaria-*attribute removed" (line 159). This guarantee applies regardless of whether the attribute exists. The underlyingremoveAttributemethod (from the ui-awesome/html-mixin dependency) is documented in the @method annotation as returning "a new instance" (line 35). The test's assertion on line 101 is therefore valid.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @tests/Global/HasDraggableTest.php:
- Around line 41-63: The test's $expected parameter is incorrectly typed as
bool|string|UnitEnum even though Attributes::render(...) always returns a string
and DraggableProvider::renderAttribute() provides strings; update the signature
of testRenderAttributesWithDraggableAttribute to declare $expected as string
(and adjust any related phpdoc or dataprovider annotation types if present) so
phpstan no longer complains and the type reflects the actual return value of
Attributes::render.
- Around line 92-114: The test's $expected parameter incorrectly includes bool
even though HasDraggable::draggable normalizes boolean inputs to the strings
'true'/'false' (and preserves enums as UnitEnum); update the
testSetDraggableAttributeValue signature so $expected is typed string|UnitEnum
(not bool|string|UnitEnum) and adjust any related phpdoc or DataProvider types
accordingly to reflect that stored attribute values are string or UnitEnum.
- Around line 7-16: The test imports the wrong Message enum namespace causing an
autoload error; update the import in HasDraggableTest to reference the correct
enum (Message from UIAwesome\Html\Attribute\Exception) instead of
UIAwesome\Html\Helper\Exception so usages like Message::VALUE_NOT_IN_LIST
resolve correctly; locate the use statement for Message in the imports at the
top of the test and replace it with the Attribute\Exception namespace.
In @tests/Support/Provider/Global/DraggableProvider.php:
- Around line 37-112: The phpdoc return shape for renderAttribute() is
incorrect: update the @phpstan-return annotation to use array<string,
array{bool|string|UnitEnum|null, mixed[], string, string}> (third element should
be string not string|UnitEnum) because
EnumDataGenerator::cases(Draggable::class, true) yields rendered HTML strings;
edit the docblock above the renderAttribute() method to replace string|UnitEnum
with string so the tuple types match the actual returned data.
🧹 Nitpick comments (4)
tests/Element/HasAltTest.php (1)
47-50: Minor: Trait ordering differs from sibling test files.This file uses
HasAltbeforeHasAttributes(alphabetical), while other Element test files (HasHrefTest, HasSrcTest, etc.) useHasAttributesbefore the specific trait. Consider aligning the trait ordering for consistency across the test suite.♻️ Optional: Align trait order with sibling tests
$instance = new class { - use HasAlt; use HasAttributes; + use HasAlt; };tests/Global/CanBeAutofocusTest.php (1)
92-111: LGTM! Thorough value storage validation.The test effectively validates that the
autofocus()method correctly stores the attribute value using data provider scenarios. The use ofGlobalAttribute::AUTOFOCUS->valueas the key ensures type-safe attribute access.Optional: Consider making the missing attribute case more explicit.
On line 108, the fallback to an empty string (
?? '') serves as a defensive measure but could potentially mask failures if the attribute is unexpectedly not set. Since all data provider test cases should have the attribute set after callingautofocus(), you might consider making this more explicit:♻️ Optional refactor for clearer assertion
- self::assertSame( - $expected, - $instance->getAttributes()[GlobalAttribute::AUTOFOCUS->value] ?? '', - $message, - ); + $attributes = $instance->getAttributes(); + self::assertArrayHasKey( + GlobalAttribute::AUTOFOCUS->value, + $attributes, + 'Autofocus attribute should be set after calling autofocus().' + ); + self::assertSame( + $expected, + $attributes[GlobalAttribute::AUTOFOCUS->value], + $message, + );This approach would catch cases where the attribute is not set when it should be, rather than silently comparing against an empty string.
tests/Global/HasDraggableTest.php (1)
116-133: Make the expected invalid-value message easier to read/debug (and ensure quote formatting matches).
The inlineimplode(...)insideexpectExceptionMessage(...)makes failures harder to interpret; extracting the allowed list to a variable helps. Also, confirm the quote formatting matches whatValidator::oneOf(...)actually produces.Proposed diff
@@ public function testThrowInvalidArgumentExceptionForSettingInvalidDraggableValue(): void { @@ + $allowed = implode('\', \'', Enum::normalizeArray(Draggable::cases())); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage( Message::VALUE_NOT_IN_LIST->getMessage( 'invalid-value', GlobalAttribute::DRAGGABLE->value, - implode('\', \'', Enum::normalizeArray(Draggable::cases())), + $allowed, ), );tests/Support/Provider/Global/ContentEditableProvider.php (1)
50-110: Reduce duplication betweenrenderAttribute()andvalues()(attribute name literal + near-identical static cases).Optional: extract the attribute name to a class constant and consider factoring the shared static scenarios to avoid drift (you already have separate render/non-render expected outputs).
Proposed refactor (no behavior change)
final class ContentEditableProvider { + private const ATTR = 'contenteditable'; + public static function renderAttribute(): array { - $enumCases = EnumDataGenerator::cases(ContentEditable::class, 'contenteditable', true); + $enumCases = EnumDataGenerator::cases(ContentEditable::class, self::ATTR, true); - $staticCase = [ + $staticCases = [ 'boolean false' => [ false, [], ' contenteditable="false"', 'Should return the attribute value after setting it.', ], // ... ]; - return [...$staticCase, ...$enumCases]; + return [...$staticCases, ...$enumCases]; } public static function values(): array { - $enumCases = EnumDataGenerator::cases(ContentEditable::class, 'contenteditable', false); + $enumCases = EnumDataGenerator::cases(ContentEditable::class, self::ATTR, false); - $staticCase = [ + $staticCases = [ 'boolean false' => [ false, [], 'false', 'Should return the attribute value after setting it.', ], // ... ]; - return [...$staticCase, ...$enumCases]; + return [...$staticCases, ...$enumCases]; } }Also applies to: 128-182
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/Values/Aria.phpsrc/Values/ElementAttribute.phptests/Element/HasAltTest.phptests/Element/HasHeightTest.phptests/Element/HasHrefTest.phptests/Element/HasReferrerpolicyTest.phptests/Element/HasSrcTest.phptests/Element/HasWidthTest.phptests/Global/CanBeAutofocusTest.phptests/Global/CanBeHiddenTest.phptests/Global/HasAccesskeyTest.phptests/Global/HasClassTest.phptests/Global/HasContentEditableTest.phptests/Global/HasDirTest.phptests/Global/HasDraggableTest.phptests/Support/Provider/Global/ContentEditableProvider.phptests/Support/Provider/Global/DraggableProvider.php
✅ Files skipped from review due to trivial changes (1)
- tests/Global/HasClassTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/Global/CanBeHiddenTest.php
- tests/Global/HasContentEditableTest.php
- src/Values/ElementAttribute.php
- tests/Global/HasAccesskeyTest.php
- tests/Global/HasDirTest.php
🧰 Additional context used
🧬 Code graph analysis (8)
tests/Support/Provider/Global/DraggableProvider.php (6)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(25-89)cases(42-64)tests/Support/Provider/Global/ContentEditableProvider.php (2)
renderAttribute(50-113)values(128-184)tests/Support/Provider/Global/AutofocusProvider.php (2)
renderAttribute(44-72)values(87-115)tests/Support/Provider/Global/ClassProvider.php (2)
renderAttribute(49-121)values(138-215)tests/Support/Provider/Global/HiddenProvider.php (2)
renderAttribute(44-72)values(87-115)src/Global/HasClass.php (1)
class(78-89)
tests/Element/HasHrefTest.php (1)
tests/Support/Provider/Element/HrefProvider.php (1)
HrefProvider(29-152)
tests/Element/HasReferrerpolicyTest.php (1)
tests/Support/Provider/Element/ReferrerpolicyProvider.php (1)
ReferrerpolicyProvider(34-148)
tests/Global/HasDraggableTest.php (3)
tests/Support/Provider/Global/DraggableProvider.php (1)
DraggableProvider(35-184)src/Global/HasDraggable.php (1)
draggable(69-78)src/Exception/Message.php (1)
getMessage(63-66)
tests/Global/CanBeAutofocusTest.php (2)
tests/Support/Provider/Global/AutofocusProvider.php (1)
AutofocusProvider(29-116)src/Global/CanBeAutofocus.php (1)
autofocus(54-57)
tests/Element/HasHeightTest.php (2)
tests/Element/HasSrcTest.php (3)
DataProviderExternal(40-59)DataProviderExternal(91-110)Group(34-111)tests/Support/Provider/Element/HeightProvider.php (1)
HeightProvider(28-127)
tests/Element/HasSrcTest.php (2)
tests/Element/HasHrefTest.php (3)
DataProviderExternal(40-59)DataProviderExternal(91-110)Group(34-111)tests/Support/Provider/Element/SrcProvider.php (1)
SrcProvider(27-126)
tests/Element/HasWidthTest.php (1)
tests/Support/Provider/Element/WidthProvider.php (1)
WidthProvider(27-126)
⏰ 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). (9)
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (18)
tests/Element/HasHrefTest.php (1)
5-11: LGTM! Consistent namespace migration and type-safe attribute key usage.The refactoring correctly:
- Updates namespace and imports to the Element context
- Uses
ElementAttribute::HREF->valuefor type-safe attribute key access- Updates the test group annotation to match the new namespace
Also applies to: 34-34, 107-107
tests/Element/HasReferrerpolicyTest.php (1)
5-12: LGTM! Properly migrated to Element namespace with consistent enum usage.The refactoring correctly updates:
- Namespace and imports to Element context
ElementAttribute::REFERRERPOLICY->valuefor attribute key access in assertions and exception messages- Test group annotation
Also applies to: 37-37, 110-110, 126-126
tests/Element/HasSrcTest.php (1)
5-11: LGTM! Clean migration to Element namespace.The changes consistently apply the refactoring pattern with updated imports, namespace, group annotation, and type-safe
ElementAttribute::SRC->valuefor attribute key access.Also applies to: 34-34, 107-107
tests/Element/HasHeightTest.php (1)
5-11: LGTM! Consistent with the Element namespace migration pattern.Properly updates namespace, imports, group annotation, and uses
ElementAttribute::HEIGHT->valuefor type-safe attribute key access.Also applies to: 34-34, 107-107
tests/Element/HasWidthTest.php (1)
5-11: LGTM! Follows the established refactoring pattern.All changes are consistent with the Element namespace migration: updated imports, namespace, group annotation, and
ElementAttribute::WIDTH->valuefor attribute key access.Also applies to: 34-34, 107-107
tests/Element/HasAltTest.php (1)
5-11: LGTM! Consistent Element namespace migration.Properly updates namespace, imports, group annotation, and uses
ElementAttribute::ALT->valuefor type-safe attribute key access.Also applies to: 34-34, 107-107
tests/Global/CanBeAutofocusTest.php (4)
1-36: LGTM! Well-structured test file with comprehensive documentation.The file header, imports, and PHPDoc are well-organized. The documentation clearly explains the test suite's purpose, coverage areas, and references to related classes. The
#[Group('global')]attribute provides good test categorization.
41-60: LGTM! Comprehensive rendering test with data provider.The test method effectively validates the rendering behavior of the autofocus attribute using an external data provider. The anonymous class approach cleanly composes the required traits for testing, and the assertion verifies the complete rendering output.
62-73: LGTM! Clear test for default state.This test appropriately validates that no attributes are set by default, ensuring the trait doesn't introduce unexpected side effects. The assertion message clearly explains the expected behavior.
75-87: LGTM! Proper immutability test.The test correctly uses
assertNotSameto verify that theautofocus()method returns a new instance, ensuring the trait maintains immutability. This is essential for validating the expected API contract.tests/Global/HasDraggableTest.php (2)
65-76: Good baseline assertion for the “not set” case.
Covers the empty-initial-state contract clearly.
78-90: Immutability check looks consistent with the trait’s fluent API.
This aligns with other “Has*” attribute traits that return a new instance on mutation.tests/Support/Provider/Global/DraggableProvider.php (1)
114-183: Provider dataset for stored values matchesHasDraggablesemantics.
Covers bool normalization, string pass-through, enum inputs, replacement, and unset (null) paths.tests/Support/Provider/Global/ContentEditableProvider.php (2)
52-53: No action required. The code correctly usesEnumDataGenerator::cases()without key collision risk: generated keys follow the"enum: {$value}"format while static cases use distinct descriptive names. Both the generated and static cases maintain the documented tuple shapearray{bool|string|UnitEnum|null, mixed[], string|UnitEnum, string}, which is consistent across all four locations (lines 52-53, 112-113, 130-131, 183-184).
14-31: Verify if empty string forcontenteditableshould follow HTML spec semantics.The provider treats
''andnullas "unset attribute" (empty rendering) across lines 67–72 and 145–150. However, per the HTML specification, an empty or missingcontenteditablevalue should be interpreted astrue(element is editable). This design appears intentional and consistent across all attributes in the library (rel, crossorigin, draggable, etc.), but warrants confirmation that this behavior is desired forcontenteditablespecifically, since it contradicts the standard.src/Values/Aria.php (3)
1-6: LGTM! Clean file structure with strict typing.The file header follows PHP best practices with strict type declarations and proper namespace organization.
7-22: Excellent documentation!The class-level documentation clearly explains the enum's purpose, intended usage patterns, and provides authoritative references to WAI-ARIA specification and MDN.
23-381: The enum implementation is complete and well-structured.The enum includes all 51 current WAI-ARIA 1.2 attributes with excellent documentation, proper string-backed enum syntax, and consistent naming conventions.
Regarding the potentially missing attributes noted in the original comment:
aria-dropeffectandaria-grabbedare deprecated in WAI-ARIA 1.1 and intentionally excluded from the enum, which is a reasonable design choice since deprecated attributes are unlikely to be used in modern applications.The omission of the
aria-prefix in the enum values (e.g.,'activedescendant'instead of'aria-activedescendant') is an appropriate design decision that allows consumers to control how the attribute names are rendered.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/Global/HasDraggableTest.php:
- Around line 78-90: The test is failing because calling
HasDraggable::draggable('') triggers validation against Draggable::cases() and
throws; change the test to pass a valid enum case value (e.g., use 'true' or
Draggable::True->value) when calling $instance->draggable(...) so the method
returns a new immutable instance and assertNotSame verifies immutability of the
draggable(...) call.
🧹 Nitpick comments (1)
tests/Global/HasDraggableTest.php (1)
116-133: Exception coverage is good; consider loosening message matching if templates are expected to evolve.If message templates/lists can change across packages/versions,
expectExceptionMessageMatches()(or checking key substrings) can reduce brittle failures while still validating behavior.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Global/HasDraggableTest.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Global/HasDraggableTest.php (4)
tests/Support/Provider/Global/DraggableProvider.php (1)
DraggableProvider(35-184)src/Global/HasDraggable.php (1)
draggable(69-78)src/Exception/Message.php (1)
getMessage(63-66)tests/Support/EnumDataGenerator.php (1)
cases(42-64)
⏰ 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). (10)
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (4)
tests/Global/HasDraggableTest.php (4)
41-63: Data-provider render assertion looks consistent withAttributes::render()usage.Covers bool/string/UnitEnum/null inputs through the provider and asserts rendered attribute output end-to-end.
65-76: Good default-state test (no attributes when unset).
92-114: Attribute value assertion is solid (including null/unset via?? '').
10-16: Remove concern about Message enum namespace mismatch—the import is correct.The test correctly imports from
UIAwesome\Html\Helper\Exception\MessagebecauseValidator::oneOf()is from the externalui-awesome/html-helperpackage (dependency in composer.json). The Helper package's Message enum containsVALUE_NOT_IN_LIST; the localsrc/Exception/Message.phpserves a different purpose and does not define validation errors. This import pattern is consistent across all attribute test files and is the correct contract.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/Values/Role.php:
- Around line 7-18: The class docblock in Role (Role.php) incorrectly says
“translate identifiers” which looks copied from translate attribute docs; update
the docblock summary and description to replace “translate identifiers” with
“role identifiers” or “role values” and remove any translate-specific wording so
it accurately describes that Role provides standardized role identifiers/values
for HTML role attributes used by tag rendering, components, and accessibility
helpers (e.g., edit the PHPDoc above the Role class and any references to
“translate identifiers”).
In @tests/Global/HasIdTest.php:
- Line 5: The namespace in HasIdTest.php is misspelled; update the namespace
declaration from "UIAwesome\Html\Attribue\Tests\Global" to
"UIAwesome\Html\Attribute\Tests\Global" and ensure any matching use/imports or
class references in this file reflect the corrected "Attribute" spelling so
autoloading and tests resolve correctly.
In @tests/Global/HasStyleTest.php:
- Around line 16-34: Update the class-level docblock for the HasStyleTest test
suite to accurately list all supported `style` value types: include `array` and
`Stringable` alongside `string`, `UnitEnum`, and `null`; specifically mention
that HasStyle::style(...) accepts `string|array|Stringable|UnitEnum|null` so the
documentation matches the trait and the StyleProvider-driven tests.
In @tests/Support/Provider/Global/TabIndexProvider.php:
- Around line 75-139: The data provider renderAttribute in the TabIndexProvider
contains duplicate test cases for negative tabindex ('integer negative' and
'negative integer'); remove one of these entries (keep either 'integer negative'
or 'negative integer') so the dataset isn't duplicated, ensuring the remaining
entry still asserts tabindex="-1" and preserving surrounding cases like 'string
negative' and 'null'.
- Around line 32-61: The phpstan return shape on invalidValues() incorrectly
includes null and the docstring mentions a float but only tests a float-like
string; update the phpstan annotation to array<string, array{int|string}>
(remove null) and either add a real float test case (5.5) to the returned array
or change the doc comment to say “float-like string” instead of “float” so the
docs match the actual test data in invalidValues().
🧹 Nitpick comments (11)
tests/Support/Provider/Global/IdProvider.php (1)
93-127: Consider DRYingvalues()/renderAttribute()to avoid future drift.
Right now the scenario list is duplicated; extracting a shared dataset (private const/private method) would reduce maintenance risk.src/Values/Language.php (1)
7-24: Be explicit about completeness/extension strategy forLanguage.
Since BCP 47 is effectively open-ended, consider documenting whether callers can pass arbitrary strings (outside the enum) or whether this enum is intended as a “common set” only.tests/Support/Provider/Global/ItemPropProvider.php (1)
45-79: Matches the established provider pattern; scenarios look correct.
Optional: factor out the duplicated scenario list betweenrenderAttribute()andvalues()to reduce maintenance overhead.Also applies to: 94-128
tests/Support/Provider/Global/ItemTypeProvider.php (1)
44-78: Consistent provider coverage for set/replace/unset semantics.
Optional: share the scenario list betweenrenderAttribute()andvalues()to avoid duplication across providers.Also applies to: 93-127
src/Global/HasTabindex.php (1)
56-65: Consider making the>= -1range constraint explicit.The current validation relies on
Validator::intLike()to reject values below-1, but this implicit dependency is not obvious. While the test suite confirms values like-2are correctly rejected, explicitly checking the range would make the intent clearer and reduce future maintenance risk.Suggested improvement (optional refactoring)
public function tabIndex(int|string|null $value): static { if ($value !== -1 && $value !== '-1' && $value !== null && Validator::intLike($value) === false) { throw new InvalidArgumentException( Message::ATTRIBUTE_INVALID_VALUE->getMessage($value, GlobalAttribute::TABINDEX->value, 'value >= -1'), ); } + + if ($value !== null && Validator::intLike($value) === true && (int) $value < -1) { + throw new InvalidArgumentException( + Message::ATTRIBUTE_INVALID_VALUE->getMessage($value, GlobalAttribute::TABINDEX->value, 'value >= -1'), + ); + } return $this->addAttribute(GlobalAttribute::TABINDEX, $value); }tests/Support/Provider/Global/SpellcheckProvider.php (1)
45-97: Solid provider coverage; consider DRYing the duplicated matrices (optional).
The cases look consistent with other global-attribute providers (replace/unset/null/empty-string coverage is good). If you want to reduce maintenance, you could derivevalues()fromrenderAttribute()(or vice versa) to avoid keeping two near-identical sets in sync.Also applies to: 112-164
tests/Global/HasStyleTest.php (1)
38-114: Tests look consistent and cover the important behaviors (rendering + immutability + stored value).tests/Support/Provider/Global/TitleProvider.php (1)
126-179: Consider adding an “enum replace existing” case tovalues()for symmetry withrenderAttribute().
You already test enum replacement in rendered output; adding the analogous stored-value case would make the provider more complete.Proposed diff
return [ 'empty string' => [ '', [], '', 'Should return an empty string when setting an empty string.', ], 'enum' => [ Status::ACTIVE, [], Status::ACTIVE, 'Should return the attribute value after setting it.', ], + 'enum replace existing' => [ + Status::INACTIVE, + ['title' => 'active'], + Status::INACTIVE, + "Should return new 'title' after replacing the existing 'title' attribute with enum value.", + ], 'null' => [ null, [], '', "Should return an empty string when the attribute is set to 'null'.", ],tests/Support/Provider/Global/RoleProvider.php (1)
107-145: Consider adding 'enum replace existing' test case tovalues()for consistency.The
renderAttribute()method includes an 'enum replace existing' test case (lines 59-64), butvalues()does not have a corresponding case. This asymmetry may result in incomplete coverage for enum replacement scenarios when testing the attribute value (as opposed to rendered output).Proposed addition
$staticCase = [ 'empty string' => [ '', [], '', 'Should return an empty string when setting an empty string.', ], + 'enum replace existing' => [ + Role::CHECKBOX, + ['role' => 'alert'], + Role::CHECKBOX, + "Should return new 'role' after replacing the existing 'role' attribute with enum value.", + ], 'null' => [ null,tests/Global/HasMicroDataTest.php (2)
44-162: LGTM!The render tests comprehensively validate each microdata attribute with data-driven test cases. The use of
bool|nullforitemScope(line 121) correctly reflects thatitemscopeis a boolean attribute, while the other microdata attributes usestring|null.The pattern of creating an anonymous class with
HasAttributesandHasMicroDatatraits is repeated across all test methods. While this aligns with the established pattern in the codebase, consider extracting it to a helper method to reduce duplication:♻️ Optional refactor to reduce duplication
private function createTestInstance(): object { return new class { use HasAttributes; use HasMicroData; }; }Then replace each instantiation with:
$instance = $this->createTestInstance();
247-365: Consider usingGlobalAttributeenum values for attribute keys.The value setting tests correctly validate attribute storage and retrieval. However, lines 266, 290, 314, 338, and 362 use hardcoded string keys (
'itemid','itemprop','itemref','itemscope','itemtype') to access the attributes array.For consistency with the
HasMicroDatatrait implementation (which usesGlobalAttribute::ITEMID, etc.) and improved maintainability, consider using the enum values instead.♻️ Refactor to use GlobalAttribute enum values
If
GlobalAttributeis a backed enum with string values, you can access the attribute keys like this:+use UIAwesome\Html\Attribute\GlobalAttribute; + // ... in testSetItemIdAttributeValue self::assertSame( $expected, - $instance->getAttributes()['itemid'] ?? '', + $instance->getAttributes()[GlobalAttribute::ITEMID->value] ?? '', $message, ); // ... in testSetItemPropAttributeValue self::assertSame( $expected, - $instance->getAttributes()['itemprop'] ?? '', + $instance->getAttributes()[GlobalAttribute::ITEMPROP->value] ?? '', $message, ); // ... in testSetItemRefAttributeValue self::assertSame( $expected, - $instance->getAttributes()['itemref'] ?? '', + $instance->getAttributes()[GlobalAttribute::ITEMREF->value] ?? '', $message, ); // ... in testSetItemScopeAttributeValue self::assertSame( $expected, - $instance->getAttributes()['itemscope'] ?? '', + $instance->getAttributes()[GlobalAttribute::ITEMSCOPE->value] ?? '', $message, ); // ... in testSetItemTypeAttributeValue self::assertSame( $expected, - $instance->getAttributes()['itemtype'] ?? '', + $instance->getAttributes()[GlobalAttribute::ITEMTYPE->value] ?? '', $message, );This ensures that if attribute names change, only the enum definition needs updating.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
src/Exception/Message.phpsrc/Global/HasEvents.phpsrc/Global/HasId.phpsrc/Global/HasLang.phpsrc/Global/HasMicroData.phpsrc/Global/HasRole.phpsrc/Global/HasSpellcheck.phpsrc/Global/HasStyle.phpsrc/Global/HasTabindex.phpsrc/Global/HasTitle.phpsrc/Global/HasTranslate.phpsrc/Values/ElementAttribute.phpsrc/Values/Event.phpsrc/Values/Language.phpsrc/Values/Role.phpsrc/Values/Translate.phptests/Global/HasEventsTest.phptests/Global/HasIdTest.phptests/Global/HasLangTest.phptests/Global/HasMicroDataTest.phptests/Global/HasRoleTest.phptests/Global/HasSpellcheckTest.phptests/Global/HasStyleTest.phptests/Global/HasTabIndexTest.phptests/Global/HasTitleTest.phptests/Global/HasTranslateTest.phptests/Support/EnumDataGenerator.phptests/Support/Provider/Global/EventProvider.phptests/Support/Provider/Global/IdProvider.phptests/Support/Provider/Global/ItemIdProvider.phptests/Support/Provider/Global/ItemPropProvider.phptests/Support/Provider/Global/ItemRefProvider.phptests/Support/Provider/Global/ItemScopeProvider.phptests/Support/Provider/Global/ItemTypeProvider.phptests/Support/Provider/Global/LangProvider.phptests/Support/Provider/Global/RoleProvider.phptests/Support/Provider/Global/SpellcheckProvider.phptests/Support/Provider/Global/StyleProvider.phptests/Support/Provider/Global/TabIndexProvider.phptests/Support/Provider/Global/TitleProvider.phptests/Support/Provider/Global/TranslateProvider.phptests/Support/Stub/Values/Status.php
🧰 Additional context used
🧬 Code graph analysis (16)
tests/Support/Provider/Global/ItemScopeProvider.php (2)
tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/ItemRefProvider.php (2)
renderAttribute(45-79)values(94-128)
tests/Global/HasEventsTest.php (3)
tests/Support/Provider/Global/EventProvider.php (1)
EventProvider(34-440)src/Global/HasEvents.php (3)
events(110-127)addEvent(75-82)removeEvent(149-154)src/Exception/Message.php (1)
getMessage(70-73)
tests/Support/Provider/Global/StyleProvider.php (1)
tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)
tests/Support/Provider/Global/TitleProvider.php (6)
tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/LangProvider.php (2)
renderAttribute(50-94)values(109-147)tests/Support/Provider/Global/RoleProvider.php (2)
renderAttribute(48-92)values(107-145)tests/Support/Provider/Global/SpellcheckProvider.php (2)
renderAttribute(45-97)values(112-164)tests/Support/Provider/Global/TabIndexProvider.php (2)
renderAttribute(75-139)values(154-212)tests/Support/Provider/Global/TranslateProvider.php (2)
renderAttribute(50-118)values(133-195)
src/Global/HasTabindex.php (1)
src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/HasTitleTest.php (2)
tests/Support/Provider/Global/TitleProvider.php (1)
TitleProvider(33-180)src/Global/HasTitle.php (1)
title(69-72)
tests/Global/HasTabIndexTest.php (2)
src/Global/HasTabindex.php (1)
tabIndex(56-65)src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/HasIdTest.php (2)
tests/Support/Provider/Global/IdProvider.php (1)
IdProvider(29-128)src/Global/HasId.php (1)
id(54-57)
tests/Global/HasStyleTest.php (3)
tests/Global/HasTitleTest.php (1)
Group(35-112)tests/Support/Provider/Global/StyleProvider.php (1)
StyleProvider(33-195)src/Global/HasStyle.php (1)
style(88-91)
tests/Global/HasLangTest.php (3)
tests/Support/Provider/Global/LangProvider.php (1)
LangProvider(35-148)src/Global/HasLang.php (1)
lang(69-74)src/Exception/Message.php (1)
getMessage(70-73)
tests/Support/Provider/Global/RoleProvider.php (2)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(24-88)cases(41-63)tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)
tests/Support/Provider/Global/IdProvider.php (1)
tests/Support/Provider/Global/ItemIdProvider.php (2)
renderAttribute(44-78)values(93-127)
tests/Global/HasMicroDataTest.php (6)
tests/Support/Provider/Global/ItemIdProvider.php (1)
ItemIdProvider(29-128)tests/Support/Provider/Global/ItemPropProvider.php (1)
ItemPropProvider(30-129)tests/Support/Provider/Global/ItemRefProvider.php (1)
ItemRefProvider(30-129)tests/Support/Provider/Global/ItemScopeProvider.php (1)
ItemScopeProvider(30-129)tests/Support/Provider/Global/ItemTypeProvider.php (1)
ItemTypeProvider(29-128)src/Global/HasMicroData.php (5)
itemId(58-61)itemProp(84-87)itemRef(110-113)itemScope(136-139)itemType(162-165)
tests/Support/Provider/Global/ItemPropProvider.php (12)
tests/Support/Provider/Global/EventProvider.php (2)
renderAttribute(105-215)values(357-439)tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/ItemIdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/ItemRefProvider.php (2)
renderAttribute(45-79)values(94-128)tests/Support/Provider/Global/ItemTypeProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/LangProvider.php (2)
renderAttribute(50-94)values(109-147)tests/Support/Provider/Global/RoleProvider.php (2)
renderAttribute(48-92)values(107-145)tests/Support/Provider/Global/SpellcheckProvider.php (2)
renderAttribute(45-97)values(112-164)tests/Support/Provider/Global/StyleProvider.php (2)
renderAttribute(48-111)values(129-194)tests/Support/Provider/Global/TabIndexProvider.php (2)
renderAttribute(75-139)values(154-212)tests/Support/Provider/Global/TitleProvider.php (2)
renderAttribute(51-108)values(126-179)tests/Support/Provider/Global/TranslateProvider.php (2)
renderAttribute(50-118)values(133-195)
tests/Support/Provider/Global/ItemTypeProvider.php (3)
tests/Support/Provider/Global/IdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/ItemIdProvider.php (2)
renderAttribute(44-78)values(93-127)tests/Support/Provider/Global/ItemRefProvider.php (2)
renderAttribute(45-79)values(94-128)
tests/Global/HasSpellcheckTest.php (3)
tests/Support/Provider/Global/SpellcheckProvider.php (1)
SpellcheckProvider(30-165)src/Global/HasSpellcheck.php (1)
spellcheck(60-69)src/Exception/Message.php (1)
getMessage(70-73)
⏰ 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). (11)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: linter / Super Linter
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
🔇 Additional comments (39)
tests/Support/Stub/Values/Status.php (1)
1-27: LGTM! Clean test stub implementation.The enum is well-structured with appropriate string backing, clear documentation, and serves its purpose as a test utility for binary state scenarios.
src/Values/ElementAttribute.php (1)
1-60: Well-structured enum with comprehensive documentation.The enum provides type-safe attribute key constants with proper MDN references. The lowercase string values correctly match HTML attribute names.
src/Exception/Message.php (1)
32-74: Good centralization of error messages.The enum design supports consistent error messaging and potential internationalization. The
getMessage()method handles variadic arguments correctly withsprintf.src/Values/Event.php (3)
369-374: Deprecated event included for completeness.The
KEY_PRESScase (onkeypress) is correctly marked as deprecated in the docblock. This is appropriate for backward compatibility, but consumers should be aware to preferKEY_DOWNorKEY_UPfor new code.
467-472: Non-standard event documented appropriately.The
MOUSE_WHEELcase is correctly marked as deprecated/non-standard. The standardWHEELevent (line 808) should be preferred.
1-809: Comprehensive event enum covering the full HTML event handler surface.The enum provides exhaustive coverage of standard, experimental, and non-standard HTML event attributes with proper MDN references throughout.
src/Values/Translate.php (1)
24-39: Correct enum for HTML translate attribute.The enum correctly defines the two explicit values (
noandyes) per the HTML specification.src/Global/HasRole.php (1)
70-75: Clean implementation with flexible enum support.The design allowing any
UnitEnumwith runtime validation againstRole::cases()provides good flexibility while maintaining standards compliance. Null handling is correctly implemented—the test suite validates that null values properly unset the attribute without throwing exceptions.src/Global/HasSpellcheck.php (1)
60-69: The null value handling is correct and does not cause validation errors. The existing test suite includes passing test cases ('null' and 'unset with null' in SpellcheckProvider) that confirm null values are properly handled byValidator::oneOf(), which must support nullable parameters across all similar attribute methods in the codebase.tests/Support/EnumDataGenerator.php (1)
45-46: Refactor improves consistency by centralizing enum normalization.Delegating to
Enum::normalizeValue()is an improvement for handling different BackedEnum value types (string and int). The explicit cast to(string)at line 82 correctly handles non-string backed enum values. However, verification of UnitEnum compatibility would require examining the externalui-awesome/html-helperpackage implementation, as no UnitEnum test cases are present in this codebase.src/Global/HasLang.php (1)
69-74: LGTM!The implementation correctly validates the
langvalue against theLanguageenum cases before setting the attribute. The immutable pattern is properly followed by delegating toaddAttribute().src/Global/HasId.php (1)
54-57: LGTM!The implementation correctly delegates to
addAttribute()and follows the immutable pattern. No validation is needed for theidattribute since it accepts arbitrary string identifiers per the HTML specification.src/Global/HasStyle.php (1)
88-91: LGTM!The implementation correctly supports multiple input types including arrays for CSS property-value pairs. The documentation clearly states that validation and conversion to CSS string format happens in the rendering layer.
src/Global/HasTitle.php (1)
69-72: LGTM!The implementation correctly handles the
titleattribute with support forStringablevalues in addition to strings and enums. Follows the established immutable pattern.src/Global/HasTranslate.php (1)
68-83: LGTM!The implementation correctly normalizes boolean values to
'yes'/'no'and handles'true'/'false'string inputs for convenience. Validation againstTranslate::cases()ensures only valid values are accepted.tests/Support/Provider/Global/ItemScopeProvider.php (1)
45-79: LGTM!The test data provider correctly covers the boolean nature of the
itemscopeattribute with appropriate test cases fortrue,false,null, replacement, and unsetting scenarios. The structure is consistent with other global attribute providers in the codebase.Also applies to: 94-128
tests/Support/Provider/Global/ItemIdProvider.php (1)
44-78: LGTM!The test data provider follows the established pattern from other global attribute providers. Test cases comprehensively cover empty string, null, replacement, and unsetting scenarios for the
itemidmicrodata attribute.Also applies to: 93-127
tests/Support/Provider/Global/IdProvider.php (1)
44-78: Provider scenarios are consistent with the suite’s conventions.
The render expectations align with existing global provider patterns (empty string, null, replace, set, unset).src/Values/Language.php (1)
25-341: Enum value formatting looks consistent (e.g.,en-US,zh-CN,es-419).
No issues spotted in the shown cases.tests/Global/HasEventsTest.php (1)
49-68: Good coverage of immutability + normalization + error messaging.
The tests exercise both bulk (events) and single (addEvent/removeEvent) paths and assert againstMessage-driven exceptions.Also applies to: 83-105, 111-152, 154-222
src/Values/Role.php (1)
24-718: Enum structure is clear and consistent for consumers (Role::…->value).
No obvious issues spotted in the shown cases.tests/Support/Provider/Global/TranslateProvider.php (1)
50-118: The review comment's collision risk concern is unfounded. EnumDataGenerator prefixes all generated keys with"enum: "(e.g.,"enum: yes","enum: no"), which is distinct from all static case keys like'boolean false','string', etc. The array spread will not cause overwrites.However, the redundancy observation is valid: both
'string'and'string boolean true'test identical scenarios (input'true'with the same expected output) in both therenderAttribute()andvalues()methods. While functionally harmless, one of these could be removed for clarity.Likely an incorrect or invalid review comment.
tests/Support/Provider/Global/ItemRefProvider.php (1)
66-70: Inconsistent input values betweenrenderAttribute()andvalues()for 'string' test case.In
renderAttribute()(line 67), the 'string' case uses'a'as input, but invalues()(line 116), the same 'string' case uses'a b'. This inconsistency may be intentional to test different scenarios, but if not, it could cause confusion during debugging.Also applies to: 115-119
tests/Support/Provider/Global/LangProvider.php (1)
1-148: LGTM!The provider follows the established pattern consistently, with comprehensive test cases for string, enum, null, and replacement scenarios. The enum data generation via
EnumDataGenerator::cases()is correctly implemented.src/Global/HasMicroData.php (1)
35-166: LGTM!The trait provides a clean, immutable API for managing HTML microdata attributes. Each method correctly delegates to
addAttribute()with the appropriateGlobalAttributeenum key, and the documentation is thorough with MDN links and usage examples.tests/Global/HasIdTest.php (1)
34-109: LGTM!The test class structure is well-organized with proper data provider integration, immutability verification, and comprehensive attribute handling tests. Once the namespace typo is fixed, this will be a solid test suite.
tests/Support/Provider/Global/StyleProvider.php (1)
33-195: LGTM!The provider offers comprehensive coverage for the style attribute, including array, string, Stringable, enum, and null scenarios. The use of inline anonymous classes for Stringable inputs is appropriate, and the expected outputs correctly use single quotes for style attribute values.
tests/Global/HasTabIndexTest.php (1)
36-132: LGTM!The test suite provides comprehensive coverage for the
HasTabindextrait, including rendering, immutability, value assignment, and importantly, validation of invalid values with proper exception handling. The use ofMessage::ATTRIBUTE_INVALID_VALUEfor exception message verification ensures consistency with the implementation.tests/Global/HasTitleTest.php (1)
1-112: LGTM!The test class is well-structured with comprehensive coverage for the
HasTitletrait:
- Proper data-provider-driven tests for rendering and value assignment
- Immutability verification using
assertNotSame- Empty state validation
- Consistent use of anonymous classes to compose the traits under test
tests/Global/HasTranslateTest.php (1)
1-134: LGTM!The test class provides thorough coverage for the
HasTranslatetrait:
- Data-provider-driven tests for rendering and value scenarios
- Proper immutability and empty-state verification
- Exception handling test validates the error message format using
Translate::cases()andEnum::normalizeArraytests/Support/Provider/Global/EventProvider.php (1)
1-439: LGTM!Comprehensive data provider with excellent coverage:
- Validates event attribute handling across multiple input types (string, Stringable, Closure, UnitEnum, null)
- Tests key normalization for
on*prefix handling- Dynamic enum case generation ensures all
Eventenum cases are tested- Well-documented PHPStan type annotations for return types
tests/Global/HasLangTest.php (1)
1-133: LGTM!The test class follows the established pattern for global attribute trait testing:
- Data-provider-driven validation for rendering and value scenarios
- Proper immutability verification
- Exception handling test validates against
Language::cases()for invalid valuessrc/Global/HasEvents.php (1)
41-155: LGTM!The trait implementation follows best practices:
- Immutable API pattern correctly implemented via
clone $this- Proper key normalization with
onprefix for all event handlers- Defensive error handling in
events()catchesTypeErrorand provides a user-friendlyInvalidArgumentException- Well-documented with comprehensive PHPDoc and usage examples
tests/Global/HasSpellcheckTest.php (1)
1-132: LGTM!The test class correctly validates the
HasSpellchecktrait:
- Data-provider-driven tests for rendering and value scenarios
- Proper immutability and empty-state verification
- Exception test correctly uses the hardcoded valid values
['false', 'true']matching the trait's validation logictests/Global/HasRoleTest.php (1)
1-133: LGTM!The test class provides comprehensive coverage for the
HasRoletrait:
- Data-provider-driven validation for rendering and value scenarios
- Proper immutability verification using
assertNotSame- Exception handling test validates against
Role::cases()for invalid valuestests/Global/HasMicroDataTest.php (4)
1-18: LGTM!File header, namespace, and imports are well-organized and correctly structured.
20-42: LGTM!Class documentation is comprehensive and clearly describes the test suite's purpose and coverage.
164-175: LGTM!The test correctly verifies that no attributes are set by default, ensuring a clean initial state.
177-245: LGTM!The immutability tests correctly verify that each setter method returns a new instance. Line 228 appropriately uses
falsefor the booleanitemScopeattribute, while the other tests use empty strings for string attributes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Values/Role.php (2)
8-18: Clarify (or support) multi-tokenrolevalues.
HTMLrolecommonly allows space-separated tokens; this enum currently models a single token. Consider documenting “single role token only” (and how to handle multiple), or adding a small helper/value-object for multi-role composition if the API supports it elsewhere.Also applies to: 24-26
54-74: Consider using native HTML elements instead of association list roles.
The rolesassociationlist,associationlistitemkey, andassociationlistitemvalueare defined in the ARIA spec but flagged as "at-risk" for reconsideration in future updates. The W3C ARIA WG recommends using native HTML<dl>,<dt>, and<dd>elements instead. Thetimerole is valid and well-defined in ARIA 1.3 and current TR, so no change needed there.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Global/HasStyle.phpsrc/Values/Role.phptests/Global/HasIdTest.phptests/Global/HasStyleTest.phptests/Support/Provider/Global/TabIndexProvider.php
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/Support/Provider/Global/TabIndexProvider.php
- tests/Global/HasIdTest.php
- tests/Global/HasStyleTest.php
⏰ 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). (11)
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (5)
src/Values/Role.php (2)
7-23: Nice addition: strongly-typedrolevalues and good in-code docs.
This should makerole=""usage more consistent and discoverable.Also applies to: 24-718
1-4: PHP constraint already correctly specifies PHP >= 8.1 for enum support.The package composer.json already declares
"php": "^8.1", which properly supports theenum Role: stringdeclaration on line 24. No changes are needed.Likely an incorrect or invalid review comment.
src/Global/HasStyle.php (3)
1-9: LGTM!The file header, namespace, and imports are well-structured. All imports are used appropriately in the method signature and implementation.
11-32: LGTM!The trait-level documentation is comprehensive and well-structured. It clearly describes the purpose, immutability guarantees, type support, and the dependency on the
addAttributemethod. The MDN reference link adds value for developers.
35-91: Implementation verified: all concerns confirmed as valid.The
GlobalAttribute::STYLEconstant exists at line 227 insrc/Values/GlobalAttribute.php. The test suite (StyleProvider) confirms array-to-CSS conversion works as documented:
- Array input:
['color' => 'red', 'font-size' => '16px']- Stored value: array unchanged
- Rendered output:
' style=\'color: red; font-size: 16px;\''The trait correctly delegates to
addAttributeand the documentation accurately describes the rendering layer responsibility. Escaping is handled byUIAwesome\Html\Helper\Attributes::render()(external dependency), as shown by the properly quoted output in test cases.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/Global/HasLangTest.php:
- Line 14: The test file imports the wrong Message class (uses
UIAwesome\Html\Helper\Exception\Message); update the import to the correct
namespace UIAwesome\Html\Attribute\Exception\Message so the test referencing
Message at line ~106 resolves; locate the use statement in HasLangTest.php and
replace the incorrect use line with the correct one for the Message class.
In @tests/Support/Provider/Global/ContentEditableProvider.php:
- Around line 48-114: The @phpstan-return docblock on the values() method
declares a 4-tuple but the datasets (including those returned by
EnumDataGenerator::cases(ContentEditable::class,
GlobalAttribute::CONTENTEDITABLE)) actually contain five elements (value,
attributes array, expected string, expected attribute fragment, and a message).
Update the docblock type to reflect a 5-tuple (include the message string as the
fifth element) so it matches the returned arrays; also optionally replace
GlobalAttribute::CONTENTEDITABLE with GlobalAttribute::CONTENTEDITABLE->value
where other providers use the enum value for consistency.
🧹 Nitpick comments (10)
tests/Support/Provider/Element/HrefProvider.php (1)
40-95: Consider making the new 5-tuple self-describing (and clarify “empty string” semantics).
Now that each row includes both an “expected value” and an “expected rendered attributes string”, it may help to (a) document the tuple positions in the PHPDoc and/or (b) rename the last 2 params in the consuming test to reduce ambiguity. Also, the “empty string” cases expect''render output; if the intended behavior is “unset attribute when value is empty string”, the message could say that explicitly.tests/Support/Provider/Element/AltProvider.php (1)
38-79: Optional: useElementAttribute::ALT->valuein the “existing attributes” fixtures too.
Right now the “existing attributes” arrays use'alt' => ...while the assertions useElementAttribute::ALT->value. Using the enum value in fixtures as well would make the tests more uniform and reduce typo risk.tests/Element/HasAltTest.php (1)
64-92: Render assertion is good; keep an eye on future attribute additions to the fixture.
Attributes::render($instance->getAttributes())asserts the entire rendered attributes string; if the anonymous test instance later gains default attributes, this may become brittle (vs asserting it contains the expected fragment).tests/Support/Provider/Element/HeightProvider.php (1)
39-80: Optional: consider usingElementAttribute::HEIGHT->valuein the “existing attributes” arrays for consistency.
Same rationale as the other providers: keeps fixtures aligned with enum-based assertions and reduces typo risk.tests/Support/Provider/Element/SrcProvider.php (1)
38-79: Optional: standardize fixture keys viaElementAttribute::SRC->value.
This would align the “existing attributes” input with the enum-based access pattern used in the updated tests.tests/Global/CanBeAutofocusTest.php (1)
81-92: Consider removing the empty string fallback for clearer type consistency.The
?? ''fallback on line 85 creates a type inconsistency:$expectedValueis declared asbool|string, but the data provider supplies onlyboolvalues. Ifautofocus($value)always sets the attribute (as suggested by the data provider), the fallback may be unnecessary and could mask missing attribute issues.♻️ Suggested refactor for stricter type handling
self::assertSame( $expectedValue, - $instance->getAttributes()[GlobalAttribute::AUTOFOCUS->value] ?? '', + $instance->getAttributes()[GlobalAttribute::AUTOFOCUS->value], $message, );This makes the test fail explicitly if the attribute is unexpectedly absent, improving test reliability.
tests/Support/Provider/Global/DraggableProvider.php (1)
50-114: Solid provider structure; consider usingGlobalAttribute::DRAGGABLE->valuefor input attribute keys to avoid typos.
The dataset shape and enum-case merge match the established provider pattern.Optional: in Line 85 and Line 106, use
GlobalAttribute::DRAGGABLE->valueinstead of the literal'draggable'for the “existing attributes” array keys, for consistency with the rest of the PR.tests/Support/Provider/Global/HiddenProvider.php (1)
42-75: Consider addingnulltest cases for consistency with other boolean attribute providers.Comparing with
ItemScopeProvider(lines 44-83 in relevant snippets), which is also a boolean attribute provider, it includes'null'and'unset with null'test cases. TheHiddenProvidercurrently only tests boolean inputs.If the
hiddentrait supportsnullfor unsetting the attribute, consider adding these cases:♻️ Suggested additional test cases
"Should return 'true' when replacing existing 'hidden' attribute with 'true'.", ], + 'null' => [ + null, + [], + '', + '', + "Should return an empty string when the attribute is set to 'null'.", + ], + 'unset with null' => [ + null, + ['hidden' => true], + '', + '', + "Should unset the 'hidden' attribute when 'null' is provided after a value.", + ], ];Note: This would also require updating the PHPStan return type on line 42 to accept
null:- @phpstan-return array<string, array{bool, mixed[], bool|string, string, string}> + @phpstan-return array<string, array{bool|null, mixed[], bool|string, string, string}>tests/Global/HasStyleTest.php (1)
36-37: Consider using consistent test group naming.This test uses
#[Group('attributes')]whileHasContentEditableTestuses#[Group('global')]. Both tests are for global HTML attributes. Consider using a consistent group name (likely'global') across all global attribute tests for easier test filtering.♻️ Suggested fix
-#[Group('attributes')] +#[Group('global')] final class HasStyleTest extends TestCasetests/Global/HasDirTest.php (1)
37-38: Consider using consistent test group naming.Similar to
HasStyleTest, this test uses#[Group('attributes')]whileHasContentEditableTestuses#[Group('global')]. Consider aligning to'global'for consistency across global attribute tests.♻️ Suggested fix
-#[Group('attributes')] +#[Group('global')] final class HasDirTest extends TestCase
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
src/Global/HasRole.phpsrc/Values/ElementAttribute.phpsrc/Values/GlobalAttribute.phptests/Element/HasAltTest.phptests/Element/HasHeightTest.phptests/Element/HasHrefTest.phptests/Element/HasReferrerpolicyTest.phptests/Element/HasSrcTest.phptests/Element/HasWidthTest.phptests/Global/CanBeAutofocusTest.phptests/Global/CanBeHiddenTest.phptests/Global/HasAccesskeyTest.phptests/Global/HasClassTest.phptests/Global/HasContentEditableTest.phptests/Global/HasDirTest.phptests/Global/HasDraggableTest.phptests/Global/HasIdTest.phptests/Global/HasLangTest.phptests/Global/HasMicroDataTest.phptests/Global/HasRoleTest.phptests/Global/HasSpellcheckTest.phptests/Global/HasStyleTest.phptests/Global/HasTabIndexTest.phptests/Global/HasTitleTest.phptests/Global/HasTranslateTest.phptests/HasCrossoriginTest.phptests/HasRelTest.phptests/Support/EnumDataGenerator.phptests/Support/Provider/CrossoriginProvider.phptests/Support/Provider/Element/AltProvider.phptests/Support/Provider/Element/HeightProvider.phptests/Support/Provider/Element/HrefProvider.phptests/Support/Provider/Element/ReferrerpolicyProvider.phptests/Support/Provider/Element/SrcProvider.phptests/Support/Provider/Element/WidthProvider.phptests/Support/Provider/Global/AccesskeyProvider.phptests/Support/Provider/Global/AutofocusProvider.phptests/Support/Provider/Global/ClassProvider.phptests/Support/Provider/Global/ContentEditableProvider.phptests/Support/Provider/Global/DirProvider.phptests/Support/Provider/Global/DraggableProvider.phptests/Support/Provider/Global/HiddenProvider.phptests/Support/Provider/Global/IdProvider.phptests/Support/Provider/Global/ItemIdProvider.phptests/Support/Provider/Global/ItemPropProvider.phptests/Support/Provider/Global/ItemRefProvider.phptests/Support/Provider/Global/ItemScopeProvider.phptests/Support/Provider/Global/ItemTypeProvider.phptests/Support/Provider/Global/LangProvider.phptests/Support/Provider/Global/RoleProvider.phptests/Support/Provider/Global/SpellcheckProvider.phptests/Support/Provider/Global/StyleProvider.phptests/Support/Provider/Global/TabIndexProvider.phptests/Support/Provider/Global/TitleProvider.phptests/Support/Provider/Global/TranslateProvider.phptests/Support/Provider/RelProvider.php
🚧 Files skipped from review as they are similar to previous changes (21)
- tests/Support/Provider/Global/ItemTypeProvider.php
- tests/Support/Provider/Global/StyleProvider.php
- tests/Global/HasIdTest.php
- tests/Support/Provider/Global/TitleProvider.php
- tests/Global/HasTranslateTest.php
- tests/Global/CanBeHiddenTest.php
- tests/Support/Provider/Global/TabIndexProvider.php
- tests/Support/Provider/Global/ItemPropProvider.php
- tests/Global/HasRoleTest.php
- tests/Support/Provider/Global/RoleProvider.php
- tests/Support/Provider/Element/ReferrerpolicyProvider.php
- tests/Support/Provider/Global/ItemRefProvider.php
- tests/Support/Provider/Global/AutofocusProvider.php
- src/Values/ElementAttribute.php
- tests/HasCrossoriginTest.php
- tests/Global/HasAccesskeyTest.php
- tests/Element/HasHrefTest.php
- tests/Support/Provider/RelProvider.php
- tests/Support/Provider/Global/ItemIdProvider.php
- tests/Global/HasTabIndexTest.php
- tests/Global/HasSpellcheckTest.php
🧰 Additional context used
🧬 Code graph analysis (21)
tests/Support/Provider/Global/DirProvider.php (1)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(24-85)cases(40-60)
tests/Support/Provider/Global/ClassProvider.php (6)
tests/Support/Provider/Global/ContentEditableProvider.php (1)
values(50-114)tests/Support/Provider/Global/DraggableProvider.php (1)
values(50-114)tests/Support/Provider/Global/StyleProvider.php (1)
__toString(54-57)tests/Support/Provider/Global/TitleProvider.php (1)
__toString(54-57)tests/Support/Provider/Global/DataProvider.php (3)
__toString(222-225)__toString(267-270)__toString(420-423)tests/Support/Provider/Global/AriaProvider.php (3)
__toString(222-225)__toString(267-270)__toString(426-429)
tests/Support/Provider/Global/IdProvider.php (2)
tests/Support/Provider/Global/ItemIdProvider.php (1)
values(44-83)tests/Support/Provider/Global/RoleProvider.php (1)
values(48-91)
tests/Global/HasClassTest.php (2)
tests/Support/Provider/Global/ClassProvider.php (1)
ClassProvider(34-138)src/Global/HasClass.php (1)
class(78-89)
tests/Support/Provider/Global/HiddenProvider.php (14)
tests/Support/Provider/CrossoriginProvider.php (1)
values(49-92)tests/Support/Provider/Element/HeightProvider.php (1)
values(43-82)tests/Support/Provider/Element/HrefProvider.php (1)
values(44-97)tests/Support/Provider/Element/ReferrerpolicyProvider.php (1)
values(49-100)tests/Support/Provider/Element/SrcProvider.php (1)
values(42-81)tests/Support/Provider/Element/WidthProvider.php (1)
values(42-81)tests/Support/Provider/Global/AccesskeyProvider.php (1)
values(44-83)tests/Support/Provider/Global/AutofocusProvider.php (1)
values(44-76)tests/Support/Provider/Global/ClassProvider.php (1)
values(51-137)tests/Support/Provider/Global/ContentEditableProvider.php (1)
values(50-114)tests/Support/Provider/Global/DirProvider.php (1)
values(48-91)tests/Support/Provider/Global/DraggableProvider.php (1)
values(50-114)tests/Support/Provider/Global/IdProvider.php (1)
values(44-83)tests/Support/Provider/Global/ItemIdProvider.php (1)
values(44-83)
tests/Support/Provider/Global/ItemScopeProvider.php (8)
tests/Support/Provider/Global/IdProvider.php (1)
values(44-83)tests/Support/Provider/Global/ItemIdProvider.php (1)
values(44-83)tests/Support/Provider/Global/ItemPropProvider.php (1)
values(45-84)tests/Support/Provider/Global/ItemRefProvider.php (1)
values(45-84)tests/Support/Provider/Global/ItemTypeProvider.php (1)
values(44-83)tests/Support/Provider/Global/LangProvider.php (1)
values(51-94)tests/Support/Provider/Global/RoleProvider.php (1)
values(48-91)tests/Support/Provider/Global/TitleProvider.php (1)
values(51-111)
tests/Global/HasDirTest.php (3)
tests/Support/Provider/Global/DirProvider.php (1)
DirProvider(33-92)src/Global/HasDir.php (1)
dir(67-72)src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/HasStyleTest.php (2)
tests/Support/Provider/Global/StyleProvider.php (1)
StyleProvider(33-126)src/Global/HasStyle.php (1)
style(88-91)
tests/Global/HasContentEditableTest.php (3)
tests/Support/Provider/Global/ContentEditableProvider.php (1)
ContentEditableProvider(35-115)src/Global/HasContentEditable.php (1)
contentEditable(69-78)src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/CanBeAutofocusTest.php (3)
tests/Support/Provider/Global/AutofocusProvider.php (1)
AutofocusProvider(29-77)src/Global/CanBeAutofocus.php (1)
autofocus(54-57)src/Global/HasClass.php (1)
class(78-89)
tests/Global/HasMicroDataTest.php (1)
src/Global/HasMicroData.php (5)
itemId(58-61)itemProp(84-87)itemRef(110-113)itemScope(136-139)itemType(162-165)
tests/Element/HasAltTest.php (1)
tests/Support/Provider/Element/AltProvider.php (1)
AltProvider(27-82)
tests/Global/HasLangTest.php (3)
tests/Support/Provider/Global/LangProvider.php (1)
LangProvider(36-95)src/Global/HasLang.php (1)
lang(69-74)src/Exception/Message.php (1)
getMessage(70-73)
tests/HasRelTest.php (1)
tests/Support/Provider/RelProvider.php (1)
RelProvider(32-91)
tests/Element/HasSrcTest.php (3)
tests/Element/HasAltTest.php (2)
DataProviderExternal(67-92)Group(34-93)tests/Element/HasHrefTest.php (2)
DataProviderExternal(67-92)Group(34-93)tests/Support/Provider/Element/SrcProvider.php (1)
SrcProvider(27-82)
tests/Global/HasDraggableTest.php (4)
tests/Support/Provider/Global/DraggableProvider.php (1)
DraggableProvider(35-115)src/Global/HasDraggable.php (1)
draggable(69-78)src/Exception/Message.php (1)
getMessage(70-73)tests/Support/EnumDataGenerator.php (1)
cases(40-60)
tests/Element/HasWidthTest.php (1)
tests/Support/Provider/Element/WidthProvider.php (1)
WidthProvider(27-82)
tests/Support/Provider/CrossoriginProvider.php (1)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(24-85)cases(40-60)
tests/Element/HasHeightTest.php (2)
tests/Element/HasAltTest.php (1)
DataProviderExternal(67-92)tests/Support/Provider/Element/HeightProvider.php (1)
HeightProvider(28-83)
tests/Support/Provider/Global/SpellcheckProvider.php (5)
tests/Support/Provider/Global/IdProvider.php (1)
values(44-83)tests/Support/Provider/Global/ItemPropProvider.php (1)
values(45-84)tests/Support/Provider/Global/ItemScopeProvider.php (1)
values(45-84)tests/Support/Provider/Global/StyleProvider.php (1)
values(51-125)tests/Support/Provider/Global/TitleProvider.php (1)
values(51-111)
tests/Global/HasTitleTest.php (2)
tests/Support/Provider/Global/TitleProvider.php (1)
TitleProvider(33-112)src/Global/HasTitle.php (1)
title(69-72)
⏰ 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). (9)
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (45)
tests/Support/Provider/Global/AccesskeyProvider.php (3)
1-30: LGTM! Well-structured provider class.The file structure, namespace, and class documentation are exemplary. The comprehensive docblock clearly explains the purpose and features of the provider, and using a
finalclass is appropriate for data providers.
31-44: LGTM! Clear method documentation.The method docblock is well-written and the PHPStan type annotation correctly describes the return structure. The documentation clearly explains the purpose and structure of the test data.
44-83: LGTM! Comprehensive test data coverage.The test cases are well-structured and cover essential scenarios: empty values, null handling, attribute replacement, and unsetting. The expected render format with the leading space is correct for HTML attribute concatenation, and the descriptive case names will aid in debugging test failures.
tests/Support/Provider/CrossoriginProvider.php (4)
5-9: LGTM!The namespace update and grouped imports are consistent with the PR's objective of centralizing attribute handling. The use of
Attributeenum for attribute keys aligns with the broader refactoring pattern.
47-47: Return type correctly reflects the expanded data structure.The 5-tuple type signature aligns with
EnumDataGenerator::cases()and properly accommodates both static cases (withstring|nullfirst element) and enum-generated cases (withUnitEnumfirst element).
51-51: LGTM!Using
Attribute::CROSSORIGINenum instead of a hardcoded string ensures type safety and aligns with the centralized attribute key pattern introduced in this PR.
53-91: LGTM!The test cases are well-structured with clear descriptive messages, and the 5-tuple format provides comprehensive coverage including rendered HTML output. The spread operator merge correctly combines static cases with the dynamically generated enum cases.
tests/HasRelTest.php (4)
5-12: LGTM! Well-organized refactoring of namespace and imports.The namespace and import path updates correctly reflect the relocation of attribute traits. The addition of the
Attributeenum import enables the type-safe, enum-based attribute key access pattern used throughout the tests, which is a best practice improvement over string literals.
37-37: LGTM! Group annotation updated consistently.The test group change from 'link' to 'attribute' aligns with the namespace reorganization and better reflects the test's scope within the broader attribute testing suite.
71-95: LGTM! Excellent use of enum-based attribute keys.The test method correctly implements the enum-based attribute key approach:
- Line 87 uses
Attribute::REL->valueinstead of the string literal'rel', improving type safety and maintainability- The dual assertions properly validate both the stored attribute value and the rendered output
- The null coalescing operator provides safe fallback for unset attributes
This refactoring aligns with best practices for avoiding magic strings in code.
97-114: LGTM! Exception handling test updated consistently.The validation test correctly uses
Attribute::REL->value(line 108) in the exception message construction, maintaining consistency with the enum-based attribute key approach throughout the test suite.tests/Support/Provider/Element/HrefProvider.php (1)
5-8: Namespace + test reference move looks consistent.
The provider now clearly targetsTests\Element\HasHrefTest, matching the PR’s Element migration.tests/Support/Provider/Element/AltProvider.php (1)
5-8: Provider relocation to Element namespace is aligned with the trait move.tests/Element/HasAltTest.php (1)
5-12: Namespace/import/group updates are coherent with the Element migration.Also applies to: 34-34
tests/Support/Provider/Element/HeightProvider.php (1)
5-8: Provider migration to Element namespace matches the PR intent.tests/Support/Provider/Element/SrcProvider.php (1)
5-8: Provider migration to Element namespace looks correct.tests/Support/Provider/Element/WidthProvider.php (1)
5-81: LGTM! Well-structured test data provider migration.The provider has been successfully migrated to the Element namespace with consistent updates:
- Return type annotation correctly reflects the new 5-tuple structure
- All test cases include the expected render attribute value
- Documentation references updated appropriately
tests/Element/HasReferrerpolicyTest.php (2)
5-12: LGTM! Clean namespace migration and import updates.The test has been successfully migrated to the Element namespace with all necessary import updates to reference the new locations of HasReferrerpolicy trait, provider, and ElementAttribute enum.
71-94: LGTM! Enhanced test assertions with dual validation.The test method now properly validates both:
- Internal attribute value via
ElementAttribute::REFERRERPOLICY->value- Rendered HTML output via
Attributes::render()This dual assertion approach provides more comprehensive coverage of attribute handling.
tests/Element/HasSrcTest.php (2)
5-11: LGTM! Consistent namespace and import migration.The test follows the same migration pattern as other Element tests, with proper namespace change and updated imports.
68-92: LGTM! Proper dual assertion for src attribute.The test correctly validates both the internal attribute storage using
ElementAttribute::SRC->valueand the rendered HTML output. The pattern is consistent with other Element tests.tests/Element/HasWidthTest.php (2)
5-11: LGTM! Consistent Element namespace migration.Namespace and imports properly updated to reference the Element-based trait and provider.
68-92: LGTM! Width attribute validation with dual assertions.The test properly validates the width attribute using
ElementAttribute::WIDTH->valuefor key access and includes render output validation. Implementation is consistent with the updated WidthProvider data structure.tests/Element/HasHeightTest.php (2)
5-11: LGTM! Final Element test migration completed consistently.Namespace and imports updated following the same pattern as the other Element tests.
68-92: LGTM! Height attribute test with proper validation.The test correctly implements dual assertions using
ElementAttribute::HEIGHT->value, maintaining consistency with the refactoring pattern applied across all Element tests.tests/Global/CanBeAutofocusTest.php (1)
38-63: LGTM! Well-structured immutability and initialization tests.The tests correctly verify both the initial empty state and the immutability guarantee of the fluent API. Using
assertNotSamefor object identity is the proper approach for testing immutability.tests/Support/Provider/Global/IdProvider.php (1)
1-84: LGTM! Well-structured data provider.The IdProvider follows the established pattern for global attribute test providers, with comprehensive test cases covering empty string, null, replacement, and unset scenarios. The documentation is clear, and the test data structure aligns perfectly with the expected 5-element format used throughout the test suite.
tests/Support/Provider/Global/TranslateProvider.php (1)
1-122: LGTM! Comprehensive translate attribute provider.The TranslateProvider correctly combines enum-generated test cases with static test cases, covering all translate attribute scenarios. The string-to-value conversions ('true'/'false' → 'yes'/'no') align with the HTML specification for the translate attribute, and the use of EnumDataGenerator ensures consistency with the Translate enum.
tests/Support/Provider/Global/ItemScopeProvider.php (1)
1-85: LGTM! Correct boolean attribute provider.The ItemScopeProvider correctly handles the itemscope boolean attribute according to HTML standards. Boolean attributes render as the attribute name without a value when true (
itemscope) and are omitted when false. The test cases comprehensively cover boolean true/false, null, replacement, and unset scenarios with appropriate type handling (bool for set values, string for unset/null).tests/Global/HasClassTest.php (1)
1-101: LGTM! Well-structured test suite.The HasClassTest follows the established testing pattern for global attribute traits, validating immutability, empty state behavior, and comprehensive value-setting scenarios through the ClassProvider data provider. The use of anonymous class composition with HasAttributes and HasClass traits is consistent with other global attribute tests, and the match expression cleanly handles the override flag for class appending vs. replacement.
src/Global/HasRole.php (1)
1-76: LGTM! Solid trait implementation with proper validation.The HasRole trait correctly implements the global role attribute with standards-compliant validation via
Validator::oneOf(). The approach of acceptingUnitEnumwhile validating againstRole::cases()is well-documented and provides flexibility for custom enums with compliant values. The immutable API, comprehensive documentation with usage examples, and links to MDN/WHATWG specifications all follow best practices.tests/Support/Provider/Global/ClassProvider.php (1)
51-137: LGTM — good coverage of append/override/null/Stringable behaviors.
The “operation list” shape (value + optional override) reads well and should compose nicely with the consuming tests.tests/Support/Provider/Global/DirProvider.php (1)
48-91: LGTM — consistent with other global attribute providers.
Static cases +EnumDataGenerator::cases(...)merge looks correct, and the tuple shape matches the stated@phpstan-return.tests/Global/HasDraggableTest.php (1)
72-115: Test may mask "unset vs present-but-null/empty" bugs; also verify theMessagetype/namespace is the intended one.
- In Line 88,
[...] ?? ''means "key missing" and "key present but null" (and often "empty-string semantics") are treated the same; if the trait is supposed to remove the key onnull, consider asserting key absence for those scenarios.use UIAwesome\Html\Helper\Exception\Message;(Line 14): please confirm this is the correctMessageenum/class yourValidatoruses forVALUE_NOT_IN_LISTmessages, otherwise the expected message assertion can be brittle.tests/Support/Provider/Global/SpellcheckProvider.php (1)
1-106: LGTM!The data provider follows the established pattern for global attribute testing. Test cases comprehensively cover boolean, string, null, replacement, and unset scenarios for the
spellcheckattribute. The PHPStan return type correctly documents the 5-tuple structure.tests/Support/Provider/Global/LangProvider.php (1)
1-95: LGTM!The provider correctly integrates with
EnumDataGeneratorto generate test cases forLanguageenum values, and includes comprehensive static test cases for edge scenarios. The merge pattern using the spread operator is clean and consistent with other enum-supporting providers in the codebase.src/Values/GlobalAttribute.php (1)
1-270: LGTM!Well-structured string-backed enum providing type-safe, standards-compliant global attribute identifiers. Each case is properly documented with MDN reference links. The
CLASS_CSSnaming convention appropriately avoids the PHP reserved keyword conflict while maintaining clarity.tests/Global/HasContentEditableTest.php (2)
38-116: LGTM!The test class provides comprehensive coverage:
- Empty state verification
- Immutability check ensuring new instances are returned
- Data-driven attribute value testing via external provider
- Invalid value exception handling with proper message verification
The test structure follows established patterns in the codebase.
14-14: Fix incorrectMessageenum import namespace.Line 14 imports
UIAwesome\Html\Helper\Exception\Message, but theMessageenum is defined in theUIAwesome\Html\Attribute\Exceptionnamespace. Change the import touse UIAwesome\Html\Attribute\Exception\Message;Likely an incorrect or invalid review comment.
tests/Global/HasStyleTest.php (1)
39-96: LGTM!The test class appropriately covers empty state, immutability, and data-driven value testing. The absence of an invalid value exception test is correct since the
HasStyle::style()method accepts any value without enum validation.tests/Global/HasDirTest.php (1)
40-115: LGTM!Comprehensive test coverage including:
- Empty state verification
- Immutability guarantees
- Data-provider-driven attribute value testing
- Invalid value exception handling with proper
Directionenum validationThe test structure is well-organized and follows established patterns.
tests/Global/HasLangTest.php (1)
40-114: LGTM! Well-structured test coverage for HasLang trait.The test suite provides comprehensive coverage:
- Empty state validation
- Immutability guarantees through instance comparison
- Data provider-driven value setting scenarios
- Exception handling for invalid language values
The test structure follows best practices and aligns with the trait's documented behavior.
tests/Support/EnumDataGenerator.php (1)
33-56: LGTM! Clean expansion to support enum-based attributes.The changes correctly:
- Broaden the
$attributeparameter to acceptstring|UnitEnum- Apply consistent normalization using
Enum::normalizeValuefor both the attribute and enum cases- Expand the return structure from 4 to 5 elements (adding a descriptive message)
- Update PHPDoc annotations to reflect the new signature and return type
This aligns well with the PR's goal of centralizing attribute handling with enums.
tests/Global/HasMicroDataTest.php (1)
45-276: LGTM! Comprehensive and well-structured microdata attribute tests.The test suite provides thorough coverage for all five microdata attributes (itemId, itemProp, itemRef, itemScope, itemType):
- Empty state verification
- Immutability guarantees for each setter
- Data provider-driven value scenarios
- Proper rendering validation through
Attributes::renderThe test structure is consistent with other global attribute tests in the PR and follows PHPUnit best practices.
tests/Global/HasTitleTest.php (1)
39-94: LGTM! Clean and consistent test coverage for HasTitle trait.The test suite follows the established pattern across global attribute tests:
- Empty state validation
- Immutability verification through instance comparison
- Data provider-driven scenarios supporting multiple types (string, Stringable, UnitEnum, null)
- Rendering validation through
Attributes::renderThe implementation is consistent with other global attribute tests in this PR and properly exercises the trait's functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/Global/HasSpellcheckTest.php:
- Around line 67-95: Rename the misspelled parameter $expectedReanderAttribute
to $expectedRenderAttribute in the test method testSetSpellcheckAttributeValue
(and update all usages inside that method, e.g. the second assert that passes
$expectedReanderAttribute to Attributes::render). Ensure the method signature
and every reference within the method body (including the assertion message
variable if used) use the corrected $expectedRenderAttribute identifier so the
parameter name matches its intended meaning.
- Line 14: The import for the Message enum is using the wrong namespace; replace
the incorrect use statement importing UIAwesome\Html\Helper\Exception\Message
with the correct namespace UIAwesome\Html\Attribute\Exception\Message in the
HasSpellcheckTest class so the Message enum resolves properly.
🧹 Nitpick comments (9)
tests/Global/HasTabIndexTest.php (4)
17-38: Consider tightening naming/coverage metadata for consistency.Minor:
HasTabIndexTest(camel “Index”) testsHasTabindex(single word). If the suite follows trait-name-mirroring elsewhere, consider aligning the class name for greppability (optional).Also optional: add PHPUnit coverage attributes (e.g.,
#[CoversClass(...)]/#[CoversNothing]) if this repo uses them consistently.
40-51: Test name says “rendering”, but it only asserts the raw attributes array.If the intent is “renders empty attributes”, consider asserting the render output too (assuming empty render is
'').Proposed diff
public function testReturnEmptyWhenTabIndexAttributeNotSet(): void { $instance = new class { use HasAttributes; use HasTabindex; }; self::assertEmpty( $instance->getAttributes(), 'Should have no attributes set when no attribute is provided.', ); + + self::assertSame( + '', + Attributes::render($instance->getAttributes()), + 'Should render no attributes when none are set.', + ); }
53-65: Immutability test: also assert the original instance remains unchanged.
assertNotSame()is good, but it doesn’t prove the original’s attributes weren’t mutated as a side effect.Proposed diff
public function testReturnNewInstanceWhenSettingTabIndexAttribute(): void { $instance = new class { use HasAttributes; use HasTabindex; }; + self::assertSame([], $instance->getAttributes()); + + $newInstance = $instance->tabIndex(1); self::assertNotSame( $instance, - $instance->tabIndex(1), + $newInstance, 'Should return a new instance when setting the attribute, ensuring immutability.', ); + + self::assertSame( + [], + $instance->getAttributes(), + 'Original instance should remain unchanged.', + ); }
98-112: Avoid hard-coding the constraint text ('value >= -1') in the expected exception message.This couples the test to message wording rather than behavior; any future tweak to the validation text will break the test even if validation remains correct. If
HasTabindex(or a validator) exposes a constant or shares the constraint message, prefer referencing that; otherwise consider asserting a stable prefix/shape of the message.tests/Global/HasTranslateTest.php (2)
18-40: Nice, consistent test scaffolding; consider a tiny factory helper to reduce repetition.The anonymous class + traits pattern is repeated 4x; a
private function newSubject(): object(returning the anonymous class) would keep future additions smaller.
54-66: Use a “definitely valid” value in the immutability test.
translate('')couples this test to the “empty string means unset” behavior; usingTranslate::YES(or'yes') makes the intent clearer and avoids false failures if validation rules tighten.Proposed diff
@@ self::assertNotSame( $instance, - $instance->translate(''), + $instance->translate(Translate::YES), 'Should return a new instance when setting the attribute, ensuring immutability.', );tests/Global/HasEventsTest.php (1)
56-59: Consider extracting the anonymous class to a helper method.The same anonymous class with
HasAttributesandHasEventstraits is instantiated in every test method. While this ensures test isolation, you could reduce duplication by extracting it to a private helper method or using asetUp()method with a property.♻️ Example refactor using a helper method
Add a helper method at the end of the test class:
private function createInstance(): object { return new class { use HasAttributes; use HasEvents; }; }Then replace each anonymous class creation with:
-$instance = new class { - use HasAttributes; - use HasEvents; -}; +$instance = $this->createInstance();However, the current approach is perfectly valid and maintains explicit clarity in each test method.
Also applies to: 72-75, 85-88, 114-117, 140-143, 160-163, 175-178, 192-195, 210-213
tests/Global/HasMicroDataTest.php (2)
58-126: Consider consolidating the five immutability tests.The five immutability tests (for
itemId,itemProp,itemRef,itemScope,itemType) follow an identical pattern. While the explicit approach is clear, you could reduce duplication by using a single parameterized test with a data provider.♻️ Example refactoring approach
#[DataProvider('microDataMethodsProvider')] public function testReturnNewInstanceWhenSettingMicroDataAttribute(string $method, mixed $value): void { $instance = new class { use HasAttributes; use HasMicroData; }; self::assertNotSame( $instance, $instance->$method($value), 'Should return a new instance when setting the attribute, ensuring immutability.', ); } public static function microDataMethodsProvider(): array { return [ 'itemId' => ['itemId', ''], 'itemProp' => ['itemProp', ''], 'itemRef' => ['itemRef', ''], 'itemScope' => ['itemScope', false], 'itemType' => ['itemType', ''], ]; }
128-276: Solid data-driven tests with minor duplication.The five value tests comprehensively verify both attribute storage and rendering using external data providers. The approach is thorough and follows good testing practices.
The slight difference in
testSetItemScopeAttributeValue(line 225:bool|string $expectedValue) compared to the string-only tests is correct, as boolean attributes render differently.While the tests are well-structured, you could reduce duplication by extracting the common assertion logic into a helper method:
♻️ Example helper method
private function assertMicroDataAttribute( string $method, mixed $value, array $attributes, GlobalAttribute $attributeKey, mixed $expectedValue, string $expectedRenderAttribute, string $message ): void { $instance = new class { use HasAttributes; use HasMicroData; }; $instance = $instance->attributes($attributes)->$method($value); self::assertSame( $expectedValue, $instance->getAttributes()[$attributeKey->value] ?? '', $message, ); self::assertSame( $expectedRenderAttribute, Attributes::render($instance->getAttributes()), $message, ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
tests/Global/HasDirTest.phptests/Global/HasEventsTest.phptests/Global/HasIdTest.phptests/Global/HasLangTest.phptests/Global/HasMicroDataTest.phptests/Global/HasRoleTest.phptests/Global/HasSpellcheckTest.phptests/Global/HasStyleTest.phptests/Global/HasTabIndexTest.phptests/Global/HasTitleTest.phptests/Global/HasTranslateTest.php
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/Global/HasIdTest.php
- tests/Global/HasDirTest.php
- tests/Global/HasRoleTest.php
- tests/Global/HasLangTest.php
- tests/Global/HasStyleTest.php
- tests/Global/HasTitleTest.php
🧰 Additional context used
🧬 Code graph analysis (4)
tests/Global/HasTranslateTest.php (3)
tests/Support/Provider/Global/TranslateProvider.php (1)
TranslateProvider(35-122)src/Global/HasTranslate.php (1)
translate(68-83)src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/HasSpellcheckTest.php (3)
tests/Support/Provider/Global/SpellcheckProvider.php (1)
SpellcheckProvider(30-106)src/Global/HasSpellcheck.php (1)
spellcheck(60-69)src/Exception/Message.php (1)
getMessage(70-73)
tests/Global/HasMicroDataTest.php (1)
src/Global/HasMicroData.php (5)
itemId(58-61)itemProp(84-87)itemRef(110-113)itemScope(136-139)itemType(162-165)
tests/Global/HasEventsTest.php (3)
tests/Support/Provider/Global/EventProvider.php (1)
EventProvider(34-440)src/Global/HasEvents.php (3)
events(110-127)addEvent(75-82)removeEvent(149-154)src/Exception/Message.php (1)
getMessage(70-73)
⏰ 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). (12)
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: mutation / PHP 8.4-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: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (23)
tests/Global/HasTabIndexTest.php (1)
67-96: Potential brittleness:Attributes::render()ordering when$attributescontains multiple keys.If
TabIndexProvider::values()includes multiple pre-set attributes, a strict string equality onAttributes::render()can become flaky unless render ordering is deterministic/canonicalized.Two options:
- Keep provider cases to one attribute at a time, or
- Assert on key presence + substring/regex for
tabindex="..."rather than full-string equality.tests/Global/HasSpellcheckTest.php (5)
17-36: LGTM!The class documentation is comprehensive and clearly describes the test suite's purpose, coverage areas, and references to related providers.
37-39: LGTM!The test class declaration follows PHPUnit best practices with proper use of the Group attribute for test organization.
40-51: LGTM!The test correctly validates that no attributes are set by default, using an appropriate assertion message.
53-65: LGTM!The test properly validates the immutability of the trait's API by confirming that setting the attribute returns a new instance.
97-114: LGTM!The test correctly validates that invalid spellcheck values trigger an
InvalidArgumentExceptionwith the appropriate error message, ensuring proper validation behavior.tests/Global/HasTranslateTest.php (2)
68-96: Data-provider test covers the important contract (stored value + render) — LGTM.
98-115: Exception assertion looks solid; keep an eye on brittleness of “exact message” checks.You’re building the expected message from the same sources (
Message, enum cases, normalization), which is good; just note this will still fail if list formatting/order changes in shared helpers.tests/Global/HasEventsTest.php (11)
1-18: LGTM! Imports are well-organized and complete.All necessary dependencies are properly imported, including PHPUnit testing infrastructure, the HasEvents trait under test, data providers, exception handling, and type definitions.
20-41: LGTM! Excellent test documentation.The class-level documentation provides comprehensive context, clearly outlining test coverage areas, validation goals, and references to related components.
45-68: LGTM! Solid rendering test with data provider integration.The test properly validates that event attributes are correctly rendered through the Attributes::render() method, using external data providers for comprehensive scenario coverage.
70-81: LGTM! Good default state verification.This test ensures that the initial state has no attributes set, providing important baseline validation.
83-105: LGTM! Comprehensive immutability validation.This test thoroughly validates that all HasEvents methods (addEvent, events, removeEvent) return new instances, ensuring proper immutability guarantees.
107-126: LGTM! Proper validation of bulk event attribute setting.The test correctly validates the events() method using data providers to cover various value types and scenarios.
128-152: LGTM! Thorough single event attribute testing.The test properly validates the addEvent() method with comprehensive type support (string, UnitEnum, Closure, Stringable, null), using data providers for broad scenario coverage.
154-171: LGTM! Proper exception handling for invalid keys.The test correctly validates that invalid event attribute keys (empty strings, booleans, integers) throw InvalidArgumentException with the appropriate message from the Message enum.
173-186: LGTM! Good validation of unsupported value types.The test properly verifies that invalid value types (stdClass) are rejected with an appropriate exception message, ensuring type safety.
188-203: LGTM! Consistent exception handling for removeEvent.The test properly validates that removeEvent() throws appropriate exceptions for invalid keys, maintaining consistency with other method validations.
205-221: LGTM! Complete exception coverage for addEvent.The test properly validates that addEvent() throws appropriate exceptions for invalid keys, completing comprehensive exception handling coverage for all HasEvents methods.
tests/Global/HasMicroDataTest.php (4)
1-20: LGTM! Clean file structure and imports.The file header, namespace, and imports are well-organized. The use of modern PHPUnit attributes and proper dependency imports sets a solid foundation for the test suite.
21-43: LGTM! Comprehensive documentation.The class documentation thoroughly describes the test suite's purpose, coverage, and dependencies. The use of the
#[Group('global')]attribute for test organization is a good practice.
45-56: LGTM! Good baseline test.This test correctly verifies the initial state when no microdata attributes are set, establishing a solid baseline for subsequent tests.
128-276: All data providers exist and are properly comprehensive. Verification confirms they cover empty strings, null values, attribute overrides, and valid microdata values according to the HTML specification.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/Support/Provider/Global/ContentEditableProvider.php:
- Around line 32-33: Update the stale copyright year in the file header comment
from 2025 to 2026; locate the block containing the copyright and license lines
(the lines with "Copyright (C) 2025 Terabytesoftw." and "BSD 3-Clause License.")
and change the year to 2026 while preserving the existing owner string and
license text.
🧹 Nitpick comments (1)
tests/Support/Provider/Global/ContentEditableProvider.php (1)
54-111: UseGlobalAttribute::CONTENTEDITABLE->valuefor initial attribute maps (avoid stringly-typed keys).The provider already depends on
GlobalAttribute; using it for the initial$attributesinput makes the dataset harder to break during refactors and avoids typos. Also$staticCasereads singular but contains multiple cases.Proposed diff
- $staticCase = [ + $staticCases = [ 'boolean false' => [ false, [], 'false', ' contenteditable="false"', 'Should return the attribute value after setting it.', ], @@ 'replace existing' => [ 'plaintext-only', - ['contenteditable' => 'false'], + [GlobalAttribute::CONTENTEDITABLE->value => 'false'], 'plaintext-only', ' contenteditable="plaintext-only"', "Should return new 'contenteditable' after replacing the existing 'contenteditable' attribute.", ], @@ 'unset with null' => [ null, - ['contenteditable' => 'true'], + [GlobalAttribute::CONTENTEDITABLE->value => 'true'], '', '', "Should unset the 'contenteditable' attribute when 'null' is provided after a value.", ], ]; - return [...$staticCase, ...$enumCases]; + return [...$staticCases, ...$enumCases];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Support/Provider/Global/ContentEditableProvider.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Support/Provider/Global/ContentEditableProvider.php (2)
tests/Support/EnumDataGenerator.php (2)
EnumDataGenerator(24-85)cases(40-60)src/Global/HasClass.php (1)
class(78-89)
⏰ 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). (1)
- GitHub Check: linter / Super Linter
🔇 Additional comments (2)
tests/Support/Provider/Global/ContentEditableProvider.php (2)
52-53: Provider is correct—no mismatch between enum case expectations and getter contract.
EnumDataGenerator::cases()correctly expects$case(theUnitEnum) as both the stored and retrieved value.HasContentEditable::contentEditable()stores the value unchanged when it's not a bool, soContentEditable::TRUEis stored as-is and retrieved as-is fromgetAttributes()['contenteditable']. The static cases (string values like'false') and enum cases (UnitEnum values) are handled consistently by the trait and will pass assertions.
69-82: Clarify the 'null' test case message—it describes "setting to null" but actually tests unsetting the attribute.The message "Should return an empty string when the attribute is set to 'null'" is misleading. Passing
nulltocontentEditable()unsetsthe attribute (removes it), not sets it to a null value. The message should reflect this behavior, for example: "Should unset the 'contenteditable' attribute when null is provided."The 'empty string' test case message is also vague about its intent—clarify whether it intentionally treats empty string as an unset operation or document the expected behavior more explicitly.
Pull Request
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.