fix(theme/default): keep inline rich text baseline aligned#84
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the CSS reset so inline rich text elements retain baseline alignment while table-related elements are still vertically aligned to the top. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe reset.css stylesheet selector list is reformatted by separating the table-related selectors ( ChangesCSS Formatting Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
vertical-alignonly has an effect on inline/inline-block/table-cell elements, consider narrowing the selector to the elements that actually need it (e.g.th, tdorcaption, th, td) instead of applying it to all table-related tags. - To keep the reset file easier to scan, you might want to keep the new
vertical-alignrule immediately adjacent to (or within) the related table reset block, matching the existing grouping/ordering conventions in this file.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `vertical-align` only has an effect on inline/inline-block/table-cell elements, consider narrowing the selector to the elements that actually need it (e.g. `th, td` or `caption, th, td`) instead of applying it to all table-related tags.
- To keep the reset file easier to scan, you might want to keep the new `vertical-align` rule immediately adjacent to (or within) the related table reset block, matching the existing grouping/ordering conventions in this file.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request separates the vertical-align property into a new CSS rule block for table-related elements in reset.css. The reviewer recommended simplifying the selector of this new rule to target only th and td elements, as vertical-align is ineffective on block-level or table-structure elements like table, caption, tbody, tfoot, thead, and tr.
| table, caption, tbody, tfoot, thead, tr, th, td { | ||
| vertical-align: top; | ||
| } |
There was a problem hiding this comment.
The vertical-align property only applies to inline-level elements and table cells (th, td). It has no effect on block-level or table-structure elements like table, caption, tbody, tfoot, thead, and tr.
Simplifying this selector to target only th, td keeps the CSS clean and avoids applying redundant properties.
th, td {
vertical-align: top;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #84 +/- ##
============================================
+ Coverage 18.13% 18.19% +0.05%
- Complexity 7854 7861 +7
============================================
Files 666 667 +1
Lines 43208 43259 +51
============================================
+ Hits 7837 7872 +35
- Misses 35371 35387 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes baseline alignment for inline rich text in the System module’s default admin theme by limiting vertical-align: top to table-related elements instead of applying it to inline text elements.
Changes:
- Removes global
vertical-align: topfrom the broad reset selector. - Adds a narrower table-element rule preserving top alignment for admin tables.
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit