Skip to content

Commit 7ebf0ab

Browse files
matt2eclaude
andcommitted
refactor: address remaining code review items for link handling
- Update test description from 'Streamdown linkSafety modal' to 'MarkdownLink'\''s LinkSafetyModal' to match actual implementation - Clean up isExternalHref import/re-export in artifactPathPolicyCore: import once at top for local use, re-export the local binding instead of a separate re-export-from-source statement - Move hardcoded English strings in LinkSafetyModal to react-i18next under components.linkSafety namespace for i18n consistency - Add comment on MessageResponse memo comparator explaining that internal state (modalUrl) is intentionally outside the comparator Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
1 parent 21096f4 commit 7ebf0ab

5 files changed

Lines changed: 20 additions & 6 deletions

File tree

ui/goose2/src/features/chat/hooks/__tests__/useArtifactLinkHandler.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe("useArtifactLinkHandler", () => {
110110
);
111111
});
112112

113-
it("does not intercept external URLs (defers to Streamdown linkSafety modal)", async () => {
113+
it("does not intercept external URLs (defers to MarkdownLink's LinkSafetyModal)", async () => {
114114
const user = userEvent.setup();
115115

116116
render(<Harness href="https://example.com" label="External" />);

ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ function hasKnownScheme(value: string): boolean {
146146
return /^[a-zA-Z][a-zA-Z\d+.-]*:/.test(value);
147147
}
148148

149-
export { isExternalHref } from "@/shared/lib/isExternalHref";
149+
export { isExternalHref };
150150

151151
function isLikelyAbsoluteFilesystemPath(candidate: string): boolean {
152152
if (/^[a-zA-Z]:[\\/]/.test(candidate)) return true;

ui/goose2/src/shared/i18n/locales/en/common.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@
4545
"title": "Environment Variables",
4646
"toggleVisibility": "Toggle value visibility"
4747
},
48+
"linkSafety": {
49+
"copied": "Copied!",
50+
"copyLink": "Copy link",
51+
"description": "You're about to visit an external website.",
52+
"openLink": "Open link",
53+
"title": "Open external link?"
54+
},
4855
"messageBranch": {
4956
"next": "Next branch",
5057
"page": "{{current}} of {{total}}",

ui/goose2/src/shared/ui/ai-elements/link-safety-modal.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useCallback, useEffect, useRef, useState } from "react";
2+
import { useTranslation } from "react-i18next";
23
import { openUrl } from "@tauri-apps/plugin-opener";
34
import { Button } from "@/shared/ui/button";
45
import {
@@ -21,6 +22,7 @@ export function LinkSafetyModal({
2122
onClose,
2223
url,
2324
}: LinkSafetyModalProps) {
25+
const { t } = useTranslation("common");
2426
const [isCopied, setIsCopied] = useState(false);
2527
const timeoutRef = useRef<number>(0);
2628

@@ -68,9 +70,9 @@ export function LinkSafetyModal({
6870
<Dialog open={isOpen} onOpenChange={handleOpenChange}>
6971
<DialogContent>
7072
<DialogHeader>
71-
<DialogTitle>Open external link?</DialogTitle>
73+
<DialogTitle>{t("components.linkSafety.title")}</DialogTitle>
7274
<DialogDescription>
73-
You&apos;re about to visit an external website.
75+
{t("components.linkSafety.description")}
7476
</DialogDescription>
7577
</DialogHeader>
7678
<div className="break-all rounded-md bg-muted p-3 font-mono text-sm">
@@ -83,10 +85,12 @@ export function LinkSafetyModal({
8385
type="button"
8486
variant="outline"
8587
>
86-
{isCopied ? "Copied!" : "Copy link"}
88+
{isCopied
89+
? t("components.linkSafety.copied")
90+
: t("components.linkSafety.copyLink")}
8791
</Button>
8892
<Button className="flex-1" onClick={handleOpen} type="button">
89-
Open link
93+
{t("components.linkSafety.openLink")}
9094
</Button>
9195
</DialogFooter>
9296
</DialogContent>

ui/goose2/src/shared/ui/ai-elements/message.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,9 @@ export const MessageResponse = memo(
414414
</LinkSafetyContext.Provider>
415415
);
416416
},
417+
// Internal state (modalUrl) is intentionally outside this comparator —
418+
// React always re-renders when local state changes regardless of memo.
419+
// If modalUrl is ever lifted to a prop, this comparator must be updated.
417420
(prevProps, nextProps) =>
418421
prevProps.children === nextProps.children &&
419422
nextProps.isAnimating === prevProps.isAnimating,

0 commit comments

Comments
 (0)