fix(elements): Improve BaseInlineElement with additional traits and update related tests.#108
Conversation
… update related tests.
WalkthroughBaseInlineElement now composes eight additional attribute-related traits (autofocus, hidden, contentEditable, dir, draggable, microdata, spellcheck, tabindex). Tests for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (3)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)src/element/BaseInlineElement.php(2 hunks)tests/html/phrasing/SpanTest.php(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/element/BaseInlineElement.php (1)
src/mixin/HasAttributes.php (1)
attributes(57-63)
⏰ 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: linter / Super Linter
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (5)
CHANGELOG.md (1)
112-112: LGTM!The changelog entry correctly documents the improvements to
BaseInlineElementand aligns with the code changes in this PR.src/element/BaseInlineElement.php (2)
10-10: LGTM! Trait additions expand inline element capabilities.The addition of
CanBeHidden,HasContentEditable,HasDir,HasDraggable,HasMicroData,HasSpellcheck, andHasTabindextraits is appropriate. These traits correspond to global HTML attributes that are valid for inline elements, and the implementation maintains alphabetical ordering in both imports and uses.Also applies to: 12-12, 14-15, 18-19, 21-21, 54-54, 58-58, 60-61, 64-64, 66-66, 69-69
9-9: CanBeAutofocus trait is intentionally implemented as a global attribute.The
CanBeAutofocustrait is explicitly documented as managing the global HTMLautofocusattribute following the HTML specification. The trait implementation references both MDN's global_attributes/autofocus and the WHATWG HTML spec, and the test suite confirms intentional support for autofocus on inline elements likeSpan.This is a valid design choice: the framework provides the API for setting autofocus on any element as a global attribute trait. Developers are responsible for ensuring they use it in compliance with HTML specifications (e.g., only on form controls or contenteditable elements in production code).
Likely an incorrect or invalid review comment.
tests/html/phrasing/SpanTest.php (2)
16-16: LGTM! Comprehensive test coverage for new attributes.The added tests properly validate rendering of all new attributes (autofocus, contentEditable, dir, draggable, hidden, microdata attributes, spellcheck, and tabindex) on Span elements. The test structure is consistent, and assertions correctly verify the expected HTML output.
Note: The
testRenderWithAutofocustest assumes thatautofocuson a<span>element is valid. As noted in theBaseInlineElement.phpreview, please verify this is intentional.Also applies to: 61-70, 94-103, 151-160, 162-171, 214-267, 327-336, 378-387
112-112: LGTM! Improved assertion messages.The assertion messages have been updated for better clarity: line 112 now correctly references "dataAttributes" attribute, and line 295 properly mentions both 'prefix()' and 'suffix()' methods.
Also applies to: 295-295
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/html/phrasing/SpanTest.php (1)
112-112: Minor wording inconsistency in assertion message.The message says "with 'dataAttributes' attribute" but refers to the
dataAttributes()method rather than a single HTML attribute. For consistency with line 295 (which uses "with both 'prefix()' and 'suffix()' methods"), consider updating to "with 'dataAttributes()' method" or "with data attributes".Apply this diff for consistency:
- "Failed asserting that element renders correctly with 'dataAttributes' attribute.", + "Failed asserting that element renders correctly with 'dataAttributes()' method.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/html/phrasing/SpanTest.php(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/html/phrasing/SpanTest.php (11)
src/html/phrasing/Span.php (1)
Span(32-53)src/tag/BaseTag.php (2)
tag(317-335)render(281-288)src/attributes/CanBeAutofocus.php (1)
autofocus(53-59)src/attributes/HasContentEditable.php (1)
contentEditable(64-83)src/attributes/HasData.php (1)
dataAttributes(68-95)src/attributes/HasDir.php (1)
dir(62-75)src/attributes/HasDraggable.php (1)
draggable(63-82)src/attributes/CanBeHidden.php (1)
hidden(53-59)src/attributes/HasMicroData.php (5)
itemId(57-68)itemProp(92-103)itemRef(127-138)itemScope(161-172)itemType(195-206)src/attributes/HasSpellcheck.php (1)
spellcheck(60-79)src/attributes/HasTabindex.php (1)
tabIndex(57-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (3)
tests/html/phrasing/SpanTest.php (3)
16-16: LGTM!The import is necessary for
testRenderWithDir()and correctly references the Direction enum.
61-70: LGTM!The new test methods provide comprehensive coverage for the additional traits added to
BaseInlineElement. Each test follows the established pattern, uses correct method names, and verifies the expected HTML output for the respective attributes.Also applies to: 94-103, 151-160, 162-171, 192-201, 214-223, 225-234, 236-245, 247-256, 258-267, 327-336, 378-387
295-295: LGTM!The updated assertion message clearly indicates that both methods are being tested and uses correct method notation.
… method assertion.
1 participant
Pull Request
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.