fix(cv): align certification org column width across rows#931
Conversation
|
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)
📝 WalkthroughWalkthroughUpdates CV template: certifications rows changed from a per-row grid to table-row/table-cell layout inside a new ChangesCertifications Layout Alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/cv-template.html (1)
306-306: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant
width: autodeclaration.The
width: autois the default value fordisplay: table-cellelements, making this declaration unnecessary.♻️ Simplify by removing redundant property
.cert-org { color: hsl(270, 70%, 45%); white-space: nowrap; - width: auto; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/cv-template.html` at line 306, Remove the redundant CSS declaration "width: auto" from the rule where it is applied (the selector that sets display: table-cell); since display: table-cell defaults to width: auto, simply delete the width: auto line to simplify the stylesheet and avoid unnecessary clutter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/cv-template.html`:
- Around line 387-389: The RTL CSS only changes .cert-year text alignment but
not the visual column order; update the RTL rules (targeting html[lang="ar"] and
the certificate row/container selectors such as .cert-row or .cert-table and the
cell selectors .cert-year, .cert-org, .cert-title) to reverse the visual order —
e.g., set the container to use flexbox/grid row-reverse (or apply explicit order
values: .cert-year {order:0}, .cert-org {order:1}, .cert-title {order:2}
switched to row-reverse so visual becomes year→org→title) and keep text
alignment adjusted for RTL; this preserves HTML structure while reversing the
column order in Arabic mode.
---
Outside diff comments:
In `@templates/cv-template.html`:
- Line 306: Remove the redundant CSS declaration "width: auto" from the rule
where it is applied (the selector that sets display: table-cell); since display:
table-cell defaults to width: auto, simply delete the width: auto line to
simplify the stylesheet and avoid unnecessary clutter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43177297-c683-41ec-aa69-9a1375195bc7
📒 Files selected for processing (1)
templates/cv-template.html
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/cv-template.html (1)
306-306: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant
width: autodeclaration.The
width: autois the default value fordisplay: table-cellelements, making this declaration unnecessary.♻️ Simplify by removing redundant property
.cert-org { color: hsl(270, 70%, 45%); white-space: nowrap; - width: auto; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/cv-template.html` at line 306, Remove the redundant CSS declaration "width: auto" from the rule where it is applied (the selector that sets display: table-cell); since display: table-cell defaults to width: auto, simply delete the width: auto line to simplify the stylesheet and avoid unnecessary clutter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/cv-template.html`:
- Around line 387-389: The RTL CSS only changes .cert-year text alignment but
not the visual column order; update the RTL rules (targeting html[lang="ar"] and
the certificate row/container selectors such as .cert-row or .cert-table and the
cell selectors .cert-year, .cert-org, .cert-title) to reverse the visual order —
e.g., set the container to use flexbox/grid row-reverse (or apply explicit order
values: .cert-year {order:0}, .cert-org {order:1}, .cert-title {order:2}
switched to row-reverse so visual becomes year→org→title) and keep text
alignment adjusted for RTL; this preserves HTML structure while reversing the
column order in Arabic mode.
---
Outside diff comments:
In `@templates/cv-template.html`:
- Line 306: Remove the redundant CSS declaration "width: auto" from the rule
where it is applied (the selector that sets display: table-cell); since display:
table-cell defaults to width: auto, simply delete the width: auto line to
simplify the stylesheet and avoid unnecessary clutter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43177297-c683-41ec-aa69-9a1375195bc7
📒 Files selected for processing (1)
templates/cv-template.html
🛑 Comments failed to post (1)
templates/cv-template.html (1)
387-389:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winRTL certification layout should reverse column order, not just text alignment.
For Arabic (RTL) mode, the current override only changes
.cert-yeartext alignment fromrighttoleft, but doesn't reverse the visual order of the table cells. In a proper RTL layout, the columns should appear in reverse visual order (year → org → title, reading right-to-left), not just reverse text alignment within each cell.🔄 Proposed fix to reverse column order in RTL mode
html[lang="ar"] .cert-year { text-align: left; } + +html[lang="ar"] .cert-item { + direction: rtl; +}This will cause the table cells to display in reverse order visually while maintaining the same HTML structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/cv-template.html` around lines 387 - 389, The RTL CSS only changes .cert-year text alignment but not the visual column order; update the RTL rules (targeting html[lang="ar"] and the certificate row/container selectors such as .cert-row or .cert-table and the cell selectors .cert-year, .cert-org, .cert-title) to reverse the visual order — e.g., set the container to use flexbox/grid row-reverse (or apply explicit order values: .cert-year {order:0}, .cert-org {order:1}, .cert-title {order:2} switched to row-reverse so visual becomes year→org→title) and keep text alignment adjusted for RTL; this preserves HTML structure while reversing the column order in Arabic mode.
Summary
Fixes #930.
Each
.cert-itemwas an independent CSS grid (minmax(0,1fr) | minmax(90px,max-content) | 44px). Long org names like "McKinsey & Company" stretched their row's second column, squeezing the title column differently per row.Switched to a table-row layout:
display: table-rowon each item withdisplay: table-cellon its children. Table semantics naturally share column widths across all rows, so the title column stays consistent.Changes
templates/cv-template.html— replaced per-row grid with table-row layout.Testing
Visual check only — this is a CSS change in a template rendered by Playwright during PDF generation. The HTML structure (
.cert-item > .cert-title + .cert-org + .cert-year) is unchanged.Summary by CodeRabbit