♿️(frontend) use semantic <p> elements in document info card#2379
♿️(frontend) use semantic <p> elements in document info card#2379Ovgodd wants to merge 1 commit into
Conversation
96c12f8 to
c41895b
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR centers the DocHeaderInfo outer Box ($align="center"), changes both Text elements to semantic paragraphs (as="p") with $margin="0", and preserves the first Text's conditional $theme and existing visibility/role separator. It also adds an Unreleased → Changed changelog entry. No public or exported APIs were modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Size Change: 0 B Total Size: 4.33 MB 📦 View Changed
|
Wrap role and last-update text in DocHeaderInfo with <p>.
c41895b to
1490520
Compare
There was a problem hiding this comment.
I don't think it is a correct semantic here, "p" means paragraph, it is means to separate block of text, it is why it has a even a native margin attribute to it. If we want to add "p" it should be more on the "Box" component so.
That said, if we want to be top notch semantic, we should use something more appropriate like dl and its friends.
@cyberbaloo what solution do you prefer / is better ? |
Purpose
Replace
<span>with<p>for the user role and last update date in the document information card (DocHeaderInfo).Proposal
<p>viaText as="p"<p>viaText as="p"$margin="0") and align the row layout to preserve the existing visual appearance