fix(email-viewer): fix date overflowing its container and overlapping download button#3909
Conversation
📝 WalkthroughWalkthroughAdjusted email preview header styling: changed vertical alignment and padding of the date element, increased background opacity and added a 1px border; a typo was introduced in the attachments border color function. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/email-viewer/email-viewer.scss (1)
28-28: 🧹 Nitpick | 🔵 Trivial
position: relativeon.email-headersis now a no-op — consider removing it.It was introduced as the containing block for the old
position: absolute.date. With the date now in-flow, no child of.email-headersuses absolute positioning, so this declaration carries no effect.♻️ Proposed cleanup
.email-headers { - position: relative; flex-shrink: 0; display: flex; flex-direction: column;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/email-viewer/email-viewer.scss` at line 28, Remove the now-unused position: relative declaration from the .email-headers rule in email-viewer.scss because .date (and other children) are in normal flow and no longer use absolute positioning; locate the .email-headers selector in the file (and verify no descendants rely on absolute positioning) and delete the position: relative line to clean up dead CSS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/email-viewer/email-viewer.scss`:
- Line 28: Remove the now-unused position: relative declaration from the
.email-headers rule in email-viewer.scss because .date (and other children) are
in normal flow and no longer use absolute positioning; locate the .email-headers
selector in the file (and verify no descendants rely on absolute positioning)
and delete the position: relative line to clean up dead CSS.
|
Good catch — removed the now-unused |
|
Nice work @adrianschmidt-bot but I'm not sure if this is looking better now when it's positioned at the bottom of the header. There was probably a reason for @Kiarokh to put it at the top in the first place. I think we should wait for his opinion and design eye :) |
|
That's a fair point! The fix solved the overflow issue, but moving the date to the bottom does change the visual hierarchy. Happy to wait for @Kiarokh's input on the best placement — he'll know what works best design-wise. 👍 |
… download button Remove absolute positioning from the date element in email headers. The date now flows naturally in the layout using flex alignment, which prevents it from escaping its container and colliding with the file-viewer's download button. fix: Lundalogik#3908
0c9ee19 to
287ade8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/email-viewer/email-viewer.scss`:
- Line 96: Fix the typo in the CSS border declaration: locate the border-bottom
rule that currently reads "border-bottom: 1px dashed rga(var(--contrast-700));"
and change the function name from "rga" to "rgba" so the declaration becomes
valid and the browser will apply the dashed border as intended.
| flex-shrink: 0; | ||
| padding: 0.5rem; | ||
| border-bottom: 1px dashed rgba(var(--contrast-700)); | ||
| border-bottom: 1px dashed rga(var(--contrast-700)); |
There was a problem hiding this comment.
Typo: rga should be rgba.
This will cause the border-bottom declaration to be invalid and ignored by the browser.
🐛 Proposed fix
- border-bottom: 1px dashed rga(var(--contrast-700));
+ border-bottom: 1px dashed rgba(var(--contrast-700));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| border-bottom: 1px dashed rga(var(--contrast-700)); | |
| border-bottom: 1px dashed rgba(var(--contrast-700)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/email-viewer/email-viewer.scss` at line 96, Fix the typo in
the CSS border declaration: locate the border-bottom rule that currently reads
"border-bottom: 1px dashed rga(var(--contrast-700));" and change the function
name from "rga" to "rgba" so the declaration becomes valid and the browser will
apply the dashed border as intended.
|
🎉 This PR is included in version 39.5.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |

What
Remove absolute positioning from the date element in email headers. The date now uses
align-self: flex-endto stay right-aligned within the flex column, instead ofposition: absolutewhich caused it to escape the container and overlap with the file-viewer's download button.Why
The date
dlusedposition: absolute; right: 0.25rem; transform: translateY(-50%)with no explicittop, causing it to float outside.email-headers. When the file-viewer rendered its download button (also absolutely positioned top-right), they'd overlap.Closes #3908
Summary by CodeRabbit