fix(email-viewer): render attachments as readonly chips#4064
Open
fix(email-viewer): render attachments as readonly chips#4064
Conversation
Switches the image attachment in the basic file-viewer example from a Wikimedia Commons URL to a Picsum Photos URL with an explicit `.jpg` extension. The previous URL had no recognizable extension on its path, which prevented the file-viewer from detecting it as an image and showed the "cannot display preview" error in some setups.
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughEmail viewer attachments moved from a semantic ul/li list to a flexbox ChangesAttachment Display Refactor
Sequence Diagram(s)sequenceDiagram
participant Client as EmailViewer
participant Parser as Filename Parser
participant Icon as Icon Helper
participant Chip as limel-chip
Client->>Parser: extract filename, extension, format size
Client->>Icon: getIconForFile(extension)
Client->>Icon: getIconFillColorForFile(extension)
Client->>Icon: getIconBackgroundColorForFile(extension)
Client->>Chip: render chip with icon, badge, tooltip (role=listitem)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-4064/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/email-viewer/email-viewer.tsx`:
- Line 234: Replace the current unreliable extension extraction (const extension
= filename.split('.').pop() ?? '') with logic that finds the last dot index and
only treats a suffix as an extension if that dot is not the first character
(i.e., dotIndex > 0); set extension to the substring after the last dot for
names like "archive.tar.gz" -> "gz", and to an empty string for names without a
dot or with a leading dot (e.g., ".gitignore"). Update the code where the local
variable extension is computed in email-viewer.tsx so downstream calls like
getIconForFile(...) and the color helper utilities receive a correct '' for
extensionless files instead of the whole filename.
🪄 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: b3b8ee50-a48c-4e53-a467-3af9720b70cd
📒 Files selected for processing (3)
src/components/email-viewer/email-viewer.scsssrc/components/email-viewer/email-viewer.tsxsrc/components/file-viewer/examples/file-viewer-basic.tsx
Kiarokh
force-pushed
the
fix/email-viewer-attachment-overflow
branch
2 times, most recently
from
d3d9ad3 to
b8e0602
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/email-viewer/email-viewer.tsx`:
- Around line 219-226: The attachments container (the div with class
"attachment-list" and role="list") isn't associated with the visible
"Attachments" span, so screen readers announce it only as "list"; add an
accessible relationship by giving the span a unique id (e.g., attachmentsLabel)
and reference that id from the list container via aria-labelledby (or
alternatively set an aria-label) so the list reads as "Attachments"—update the
JSX around the span and the div where attachments.map and renderAttachment are
used to include this id/aria-labelledby pairing.
🪄 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: 96264626-3eae-430e-bff4-64355246f588
📒 Files selected for processing (2)
src/components/email-viewer/email-viewer.scsssrc/components/email-viewer/email-viewer.tsx
Comment on lines
219
to
226
| <span>{label}</span> | ||
| <ul> | ||
| <div class="attachment-list" role="list"> | ||
| {attachments.map((attachment, index) => | ||
| this.renderAttachment(attachment, index) | ||
| )} | ||
| </ul> | ||
| </div> | ||
| </div> | ||
| ); |
Contributor
There was a problem hiding this comment.
Accessibility: associate the "Attachments" label with the role="list" container.
Without aria-labelledby, screen readers announce the container as just "list" and drop the "Attachments" context entirely. Linking the span to the list fixes this with minimal code.
♿ Proposed fix
- <div class="attachments">
- <span>{label}</span>
- <div class="attachment-list" role="list">
+ <div class="attachments">
+ <span id="attachments-label">{label}</span>
+ <div class="attachment-list" role="list" aria-labelledby="attachments-label">🤖 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 `@src/components/email-viewer/email-viewer.tsx` around lines 219 - 226, The
attachments container (the div with class "attachment-list" and role="list")
isn't associated with the visible "Attachments" span, so screen readers announce
it only as "list"; add an accessible relationship by giving the span a unique id
(e.g., attachmentsLabel) and reference that id from the list container via
aria-labelledby (or alternatively set an aria-label) so the list reads as
"Attachments"—update the JSX around the span and the div where attachments.map
and renderAttachment are used to include this id/aria-labelledby pairing.
Long filenames and verbose mime-type strings in the attachment list could overflow their card and bleed outside of the surrounding layout. Replaces the bespoke attachment cards with readonly limel-chip elements, each carrying: - a file-type icon (reusing the extension-to-icon map already used by limel-file, no duplication) - the filename as the chip text (single-line ellipsized via the chip's built-in truncation) - the file size as the chip's badge - the full filename and mime type in the host title attribute, so users can still see the verbose details on hover The list container is a plain div with role=list, and each chip carries role=listitem, since custom elements cannot be direct children of a ul.
Kiarokh
force-pushed
the
fix/email-viewer-attachment-overflow
branch
from
b8e0602 to
8bf4091
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
limel-email-viewer.limel-chipcarrying a file-type icon (reusing the same extension→icon map thatlimel-fileuses, no duplication), the filename as the chip text (single-line ellipsis on overflow), and the file size as the chip's badge. The full filename + mime type live in the host'stitleattribute for hover.role="list"+role="listitem"on each chip since custom elements can't be direct<ul>children.docs(file-viewer)swap of the image URL in the basic example to a Picsum URL with an explicit.jpg.fix: #4063
Test plan
.emlwith multiple attachments and verify the chip list renders with type icons and size badges.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Style