Skip to content

Fix/4060 download button#4145

Open
LucyChyzhova wants to merge 2 commits into
mainfrom
fix/4060-download-button
Open

Fix/4060 download button#4145
LucyChyzhova wants to merge 2 commits into
mainfrom
fix/4060-download-button

Conversation

@LucyChyzhova

@LucyChyzhova LucyChyzhova commented Jun 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

fix: #4070

Summary by CodeRabbit

Bug Fixes

  • Prevented duplicate download buttons from appearing in the file viewer when fallback content is displayed
  • Adjusted button positioning for improved layout alignment

Review:

  • Commits are atomic
  • Commits have the correct type for the changes made
  • Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

  • Chrome
  • Edge
  • Firefox

Linux:

  • Chrome
  • Firefox

macOS:

  • Chrome
  • Firefox
  • Safari

Mobile:

  • Chrome on Android
  • iOS

@LucyChyzhova
fix(file-viewer): avoid rendering two download buttons
be94034 
When `allowDownload` is enabled and a text or email file cannot be
rendered, the fallback "unsupported file type" message rendered its own
download button on top of the one already shown in the controls bar,
resulting in two download buttons.
@LucyChyzhova
fix(ui-controls): avoid overlapping buttons
e568ae2
@coderabbitai

coderabbitai Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

limel-file-viewer fixes two issues: duplicate download buttons in unsupported-filetype fallback UI and overlapping button bar positioning. renderNoFileSupportMessage gains an optional withDownloadButton flag; renderText and renderEmail pass false to suppress the inline download when a toolbar is already present. The .buttons CSS positioning switches from top to bottom.

Changes

file-viewer duplicate download and button positioning fixes

Layer / File(s) Summary
renderNoFileSupportMessage conditional download button
src/components/file-viewer/file-viewer.tsx		 renderNoFileSupportMessage accepts an optional withDownloadButton parameter (default true) and guards the download button behind it. renderText passes false to suppress the button; renderEmail wraps its fallback in a <div slot="fallback"> and also passes false.
.buttons vertical positioning
src/components/file-viewer/partial-styles/ui-controls.scss		 The .buttons bar's vertical anchor is changed from top: 0.25rem to bottom: 0.25rem, moving the toolbar to the bottom-right to avoid collisions with consumer-placed top-right controls.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • Kiarokh
  • FredrikWallstrom
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title references issue #4060, but the PR is actually addressing issue #4070. This is misleading, as the title does not accurately reflect the actual issue being fixed. Update the PR title to reference issue #4070, such as 'Fix #4070: avoid overlapping buttons and duplicate download button' or similar.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR successfully addresses both problems identified in issue #4070: preventing duplicate download buttons in unsupported file fallbacks and repositioning toolbar buttons to avoid overlaps.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the two problems identified in issue #4070; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/4060-download-button

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-4145/

@LucyChyzhova

LucyChyzhova commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

The PR title references issue #4060, but the PR is actually addressing issue #4070. This is misleading, as the title does not accurately reflect the actual issue being fixed.

yeah, missed the number , should be 4070 instead...

ha2818an
ha2818an approved these changes Jun 29, 2026
View reviewed changes

@ha2818an ha2818an left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified by linking local lime-element to local lime-bb and checking the example docs. I'd say the comments are nits so approving for now.

Comment thread src/components/file-viewer/file-viewer.tsx
Comment on lines +338 to +342
* @param withDownloadButton whether to render a download button as a
* recovery option. Should be `false` when the message is shown as
* fallback content alongside the buttons bar, which already renders its
* own download button, to avoid showing two download buttons.
*/

@ha2818an ha2818an Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc says the buttons bar "already renders its own download button," but it only does so when allowDownload is true. If we want to be precise maybe "...which already renders the download button when enabled" so it doesn't imply the button is unconditional.

Comment thread src/components/file-viewer/file-viewer.tsx
* fallback content alongside the buttons bar, which already renders its
* own download button, to avoid showing two download buttons.
*/
private renderNoFileSupportMessage = (withDownloadButton = true) => {

@ha2818an ha2818an Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adressing to get it cleared

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ha2818an ha2818an ha2818an approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

file-viewer: overlapping buttons and duplicate download in unsupported fallback

2 participants

@LucyChyzhova @ha2818an