fix(elements): Simplify attributes in BaseInline class and improve Span element with specific traits and update related tests.#112
Conversation
…`Span` element with specific traits and update related tests.
WalkthroughThe changes redistribute trait usage between BaseInline and Span classes. BaseInline removes support for CanBeAutofocus, HasContentEditable, HasDraggable, HasMicroData, HasSpellcheck, and HasTabindex traits. Span now directly implements four of these traits—HasContentEditable, HasDraggable, HasMicroData, and HasTabindex. Tests are updated to reflect trait removal. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/html/phrasing/Span.php (1)
7-8: Localizing rich attribute traits ontoSpanlooks good—ensure consistency across inline elementsWiring
HasContentEditable,HasDraggable,HasMicroData, andHasTabindexdirectly intoSpankeeps those capabilities after they were removed fromBaseInlineand matches the existing tests (contentEditable(),draggable(),item*(),tabIndex()).If other inline elements should expose the same attributes, consider applying these traits (or a shared composite trait) there as well so the inline API surface stays predictable across elements.
Also applies to: 35-38
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)src/element/BaseInline.php(1 hunks)src/html/phrasing/Span.php(2 hunks)tests/html/phrasing/SpanTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/element/BaseInline.php (1)
src/mixin/HasAttributes.php (1)
attributes(57-63)
src/html/phrasing/Span.php (1)
src/mixin/HasAttributes.php (1)
attributes(57-63)
🔇 Additional comments (3)
CHANGELOG.md (1)
116-116: Changelog entry is consistent and clearThe Fix #112 line accurately reflects the code changes (BaseInline simplification, Span traits, and tests) and follows the existing changelog style.
tests/html/phrasing/SpanTest.php (1)
44-47: Additional#[Group('span')]improves test targeting; Span tests align with new trait setAdding the dedicated
spangroup is useful for focused runs and is consistent with the existinghtml/phrasinggroups. The remaining tests exercise the attributes still available onSpan(contentEditable, draggable, microdata, tabindex) after the BaseInline simplification; autofocus and spellcheck coverage was correctly removed alongside those APIs.If
Span::tag()->autofocus()orSpan::tag()->spellcheck()were ever part of the public surface, consider whether their removal should be called out as a BC note in docs/release notes even if they were deemed incorrect for<span>.src/element/BaseInline.php (1)
7-8: BaseInline trait reduction is properly scoped—only Span extends it and restores needed traitsThe narrowing of BaseInline's trait surface (removing
CanBeAutofocus,HasContentEditable,HasDraggable,HasMicroData,HasSpellCheck,HasTabindex) is intentional and safe. Only one production class extendsBaseInline:Span, which explicitly restoresHasContentEditable,HasDraggable,HasMicroData, andHasTabindexat line 7. Span's tests confirm it uses these restored traits and does not requireCanBeAutofocusorHasSpellcheck. The stub testTagclass doesn't rely on any dropped traits. This refactoring is sound—no breaking changes or missed dependencies.
Pull Request
Summary by CodeRabbit
Release Notes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.