Add href to download iso button#3114
Add href to download iso button#3114openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
WalkthroughUpdated BotMessage download button to render as an anchor with href, adjusted onClick to receive the event, prevent default navigation, and invoke saveAs(url) within try/catch with error logging. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AnchorButton as Button (anchor)
participant Handler as onClick Handler
participant FileSaver as saveAs
participant Console as Logger
User->>AnchorButton: Click
AnchorButton->>Handler: onClick(e)
Handler->>Handler: e.preventDefault()
Handler->>FileSaver: saveAs(url)
alt Error
Handler->>Console: console.error(err)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
the
approved
openshift-ci
bot
added
size/XS
|
/cherry-pick releases/v0.1-chatbot |
|
@rawagner: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx (1)
121-129: Allow modifier-clicks and add a download fallbackTo preserve Ctrl/Cmd-Click (and other modifier-assisted navigation) while still handling plain clicks and providing a fallback if
saveAsfails, update theonClickin
libs/chatbot/lib/components/ChatBot/BotMessage.tsx(around line 124):onClick={(e) => { - e.preventDefault(); - try { - saveAs(url); - } catch (error) { - // eslint-disable-next-line - console.error('Download failed: ', error); - } + // Let modifier-clicks (e.g. Ctrl/Cmd+Click) open in a new tab + if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) { + return; + } + e.preventDefault(); + try { + saveAs(url); + } catch (error) { + // eslint-disable-next-line + console.error('Download failed:', error); + // Fallback: open the file URL so the browser can download it + void window.open(url, '_blank', 'noopener,noreferrer'); + } }}
- Targets the existing
<Button component="a" href={url}>…with the download icon- Preserves default navigation on modifier-clicks
- Falls back to
window.openwhensaveAsthrows
🧹 Nitpick comments (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx (1)
131-133: Optional: add the download attribute to hint browsers to download.For same-origin URLs, adding download can improve native download behavior and filename handling. Browsers ignore it for cross-origin URLs, so it’s safe but optional.
- component="a" - href={url} + component="a" + href={url} + download
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx(2 hunks)
🔇 Additional comments (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx (1)
131-133: Good change: anchor + href improves semantics and UX.Rendering the Button as an anchor with href aligns with link semantics, enables context-menu actions (e.g., “Copy link”, “Open in new tab”), and meets the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx (1)
121-129: Preserve native link behavior; only prevent default on unmodified left-clicks and provide a robust fallbackPreventing default on every click breaks expected UX (open-in-new-tab via Ctrl/Cmd-click, middle-click, keyboard modifiers). Restrict preventDefault to simple primary-button clicks without modifiers. Also, provide a filename to saveAs and a safe fallback if saveAs fails.
Apply this diff:
- onClick={(e) => { - e.preventDefault(); - try { - saveAs(url); - } catch (error) { - // eslint-disable-next-line - console.error('Download failed: ', error); - } - }} + onClick={(e) => { + // Only intercept simple left-clicks without modifiers. + if ( + e.button !== 0 || + e.altKey || + e.ctrlKey || + e.metaKey || + e.shiftKey + ) { + return; // allow native browser handling + } + e.preventDefault(); + try { + // Provide a best-effort filename derived from the URL. + const name = (() => { + try { + const u = new URL(url, window.location.href); + return u.pathname.split('/').pop() || undefined; + } catch { + return undefined; + } + })(); + saveAs(url, name); + } catch (error) { + // eslint-disable-next-line no-console + console.error('Download failed: ', error); + // Fallback: open the link if it's safe. + if (typeof isSafeUrl === 'function' && isSafeUrl(url)) { + window.open(url, '_blank', 'noopener,noreferrer'); + } + } + }}Add the helper referenced above (outside of this block, e.g., near the top or within the component scope):
function isSafeUrl(href: string): boolean { try { const proto = new URL(href, window.location.origin).protocol; return proto === 'https:' || proto === 'http:' || proto === 'blob:'; } catch { return false; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: format
- GitHub Check: lint
openshift-merge-bot
bot
merged commit 28f715e
into
openshift-assisted:master
|
@rawagner: new pull request created: #3115 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
3 participants
Summary by CodeRabbit
Bug Fixes
Refactor