From dad448afd2564191313515735b2be4641b6544a3 Mon Sep 17 00:00:00 2001 From: Trung Dang Date: Thu, 22 May 2025 11:41:18 +0700 Subject: [PATCH] refactor(marketplace): some UI adjustments (#13) * refactor(marketplace): add installed tabs * fix: missing settings button * refactor(marketplace): better card UI * refactor(marketplace): better error message for sources * tests(marketplace): item card and source config * refactor(marketplace): colocate local states * refactor(marketplace): simplify tabs * test: marketplace view --------- Co-authored-by: elianiva <51877647+elianiva@users.noreply.github.com> --- src/i18n/locales/en/marketplace.json | 20 +- .../marketplace/MarketplaceListView.tsx | 33 +- .../MarketplaceSourcesConfigView.tsx | 155 +++++- .../marketplace/MarketplaceView.tsx | 106 ++-- .../MarketplaceViewStateManager.ts | 6 +- .../__tests__/MarketplaceListView.test.tsx | 30 +- .../MarketplaceSourcesConfig.test.tsx | 161 +++++- .../__tests__/MarketplaceView.test.tsx | 480 ++++++++++++++++++ .../components/MarketplaceItemActionsMenu.tsx | 52 +- .../components/MarketplaceItemCard.tsx | 114 +++-- .../__tests__/MarketplaceItemCard.test.tsx | 227 ++++++++- .../src/i18n/locales/ca/marketplace.json | 60 ++- .../src/i18n/locales/de/marketplace.json | 67 ++- .../src/i18n/locales/en/marketplace.json | 19 +- .../src/i18n/locales/es/marketplace.json | 67 ++- .../src/i18n/locales/fr/marketplace.json | 67 ++- .../src/i18n/locales/hi/marketplace.json | 65 ++- .../src/i18n/locales/it/marketplace.json | 59 ++- .../src/i18n/locales/ja/marketplace.json | 61 ++- .../src/i18n/locales/ko/marketplace.json | 61 ++- .../src/i18n/locales/nl/marketplace.json | 102 ++++ .../src/i18n/locales/pl/marketplace.json | 59 ++- .../src/i18n/locales/pt-BR/marketplace.json | 52 +- .../src/i18n/locales/ru/marketplace.json | 101 ++++ .../src/i18n/locales/tr/marketplace.json | 58 ++- .../src/i18n/locales/vi/marketplace.json | 52 +- .../src/i18n/locales/zh-CN/marketplace.json | 52 +- .../src/i18n/locales/zh-TW/marketplace.json | 52 +- 28 files changed, 1888 insertions(+), 550 deletions(-) create mode 100644 webview-ui/src/components/marketplace/__tests__/MarketplaceView.test.tsx create mode 100644 webview-ui/src/i18n/locales/nl/marketplace.json create mode 100644 webview-ui/src/i18n/locales/ru/marketplace.json diff --git a/src/i18n/locales/en/marketplace.json b/src/i18n/locales/en/marketplace.json index c58b121f5b..7d9728b03a 100644 --- a/src/i18n/locales/en/marketplace.json +++ b/src/i18n/locales/en/marketplace.json @@ -27,11 +27,6 @@ "installButton": "Install", "cancelButton": "Cancel" }, - "install-sidebar": { - "title": "Install {{itemName}}", - "installButton": "Install", - "cancelButton": "Cancel" - }, "filters": { "search": { "placeholder": "Search marketplace..." @@ -78,19 +73,20 @@ "current": { "title": "Current Sources", "empty": "No marketplace sources added yet.", - "emptyHint": "Add a source above to browse marketplace items." - }, - "current": { + "emptyHint": "Add a source above to browse marketplace items.", "refresh": "Refresh source", "remove": "Remove source" } }, - "tabs": { - "browse": "Browse", - "sources": "Sources" - }, "title": "Marketplace" }, + "done": "Done", + "refresh": "Refresh", + "tabs": { + "installed": "Installed", + "browse": "Browse", + "settings": "Settings" + }, "items": { "refresh": { "refreshing": "Refreshing marketplace items..." diff --git a/webview-ui/src/components/marketplace/MarketplaceListView.tsx b/webview-ui/src/components/marketplace/MarketplaceListView.tsx index 4f5198530d..83f0d22251 100644 --- a/webview-ui/src/components/marketplace/MarketplaceListView.tsx +++ b/webview-ui/src/components/marketplace/MarketplaceListView.tsx @@ -1,3 +1,4 @@ +import * as React from "react" import { Input } from "@/components/ui/input" import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select" import { Button } from "@/components/ui/button" @@ -13,24 +14,23 @@ export interface MarketplaceListViewProps { stateManager: MarketplaceViewStateManager allTags: string[] filteredTags: string[] - tagSearch: string - setTagSearch: (value: string) => void - isTagPopoverOpen: boolean - setIsTagPopoverOpen: (value: boolean) => void + showInstalledOnly?: boolean } export function MarketplaceListView({ stateManager, allTags, filteredTags, - tagSearch, - setTagSearch, - isTagPopoverOpen, - setIsTagPopoverOpen, + showInstalledOnly = false, }: MarketplaceListViewProps) { const [state, manager] = useStateManager(stateManager) const { t } = useAppTranslation() - const items = state.displayItems || [] + const [isTagPopoverOpen, setIsTagPopoverOpen] = React.useState(false) + const [tagSearch, setTagSearch] = React.useState("") + const allItems = state.displayItems || [] + const items = showInstalledOnly + ? allItems.filter((item) => state.installedMetadata.project[item.id] || state.installedMetadata.global[item.id]) + : allItems const isEmpty = items.length === 0 return ( @@ -142,7 +142,7 @@ export function MarketplaceListView({ }, }) } - className="shadow-none bg-vscode-input-background px-2"> + className="shadow-none bg-vscode-dropdown-background px-2"> {state.sortConfig.order === "asc" ? "↑" : "↓"} @@ -176,7 +176,7 @@ export function MarketplaceListView({ )} - + setIsTagPopoverOpen(open)}> - + e.stopPropagation()}>
e.preventDefault()}> + onMouseDown={(e) => { + e.stopPropagation() + e.preventDefault() + }}> {state.filters.tags.includes(tag) ? ( ) : ( @@ -265,7 +270,7 @@ export function MarketplaceListView({
- {state.isFetching && ( + {state.isFetching && isEmpty && (
diff --git a/webview-ui/src/components/marketplace/MarketplaceSourcesConfigView.tsx b/webview-ui/src/components/marketplace/MarketplaceSourcesConfigView.tsx index 2ef4bcbc46..88f81aa389 100644 --- a/webview-ui/src/components/marketplace/MarketplaceSourcesConfigView.tsx +++ b/webview-ui/src/components/marketplace/MarketplaceSourcesConfigView.tsx @@ -6,7 +6,7 @@ import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" import { Checkbox } from "@/components/ui/checkbox" import { useStateManager } from "./useStateManager" -import { validateSource } from "@roo/shared/MarketplaceValidation" +import { validateSource, ValidationError } from "@roo/shared/MarketplaceValidation" import { cn } from "@src/lib/utils" export interface MarketplaceSourcesConfigProps { @@ -19,6 +19,62 @@ export function MarketplaceSourcesConfig({ stateManager }: MarketplaceSourcesCon const [newSourceUrl, setNewSourceUrl] = useState("") const [newSourceName, setNewSourceName] = useState("") const [error, setError] = useState("") + const [fieldErrors, setFieldErrors] = useState<{ + name?: string + url?: string + }>({}) + + // Check if name contains emoji characters + const containsEmoji = (str: string): boolean => { + // Simple emoji detection using common emoji ranges + // This avoids using Unicode property escapes which require ES2018+ + return ( + /[\ud83c\ud83d\ud83e][\ud000-\udfff]/.test(str) || // Common emoji surrogate pairs + /[\u2600-\u27BF]/.test(str) || // Misc symbols and pictographs + /[\u2300-\u23FF]/.test(str) || // Miscellaneous Technical + /[\u2700-\u27FF]/.test(str) || // Dingbats + /[\u2B50\u2B55]/.test(str) || // Star, Circle + /[\u203C\u2049\u20E3\u2122\u2139\u2194-\u2199\u21A9\u21AA]/.test(str) + ) // Punctuation + } + + // Validate input fields without submitting + const validateFields = () => { + const newErrors: { name?: string; url?: string } = {} + + // Validate name if provided + if (newSourceName) { + if (newSourceName.length > 20) { + newErrors.name = t("marketplace:sources.errors.nameTooLong") + } else if (containsEmoji(newSourceName)) { + newErrors.name = t("marketplace:sources.errors.emojiName") + } else { + // Check for duplicate names + const hasDuplicateName = state.sources.some( + (source) => source.name && source.name.toLowerCase() === newSourceName.toLowerCase(), + ) + if (hasDuplicateName) { + newErrors.name = t("marketplace:sources.errors.duplicateName") + } + } + } + + // Validate URL + if (!newSourceUrl.trim()) { + newErrors.url = t("marketplace:sources.errors.emptyUrl") + } else { + // Check for duplicate URLs + const hasDuplicateUrl = state.sources.some( + (source) => source.url.toLowerCase().trim() === newSourceUrl.toLowerCase().trim(), + ) + if (hasDuplicateUrl) { + newErrors.url = t("marketplace:sources.errors.duplicateUrl") + } + } + + setFieldErrors(newErrors) + return Object.keys(newErrors).length === 0 + } const handleAddSource = () => { const MAX_SOURCES = 10 @@ -26,11 +82,27 @@ export function MarketplaceSourcesConfig({ stateManager }: MarketplaceSourcesCon setError(t("marketplace:sources.errors.maxSources", { max: MAX_SOURCES })) return } + + // Clear previous errors + setError("") + + // Perform quick validation first + if (!validateFields()) { + // If we have specific field errors, show the first one as the main error + if (fieldErrors.url) { + setError(fieldErrors.url) + } else if (fieldErrors.name) { + setError(fieldErrors.name) + } + return + } + const sourceToValidate: MarketplaceSource = { - url: newSourceUrl, - name: newSourceName || undefined, + url: newSourceUrl.trim(), + name: newSourceName.trim() || undefined, enabled: true, } + const validationErrors = validateSource(sourceToValidate, state.sources) if (validationErrors.length > 0) { const errorMessages: Record = { @@ -42,7 +114,34 @@ export function MarketplaceSourcesConfig({ stateManager }: MarketplaceSourcesCon "name:nonvisible": "marketplace:sources.errors.nonVisibleCharsName", "name:duplicate": "marketplace:sources.errors.duplicateName", } - const error = validationErrors[0] + + // Group errors by field for better user feedback + const fieldErrorMap: Record = {} + for (const error of validationErrors) { + if (!fieldErrorMap[error.field]) { + fieldErrorMap[error.field] = [] + } + fieldErrorMap[error.field].push(error) + } + + // Update field-specific errors + const newFieldErrors: { name?: string; url?: string } = {} + if (fieldErrorMap.name) { + const error = fieldErrorMap.name[0] + const errorKey = `name:${error.message.toLowerCase().split(" ")[0]}` + newFieldErrors.name = t(errorMessages[errorKey] || error.message) + } + + if (fieldErrorMap.url) { + const error = fieldErrorMap.url[0] + const errorKey = `url:${error.message.toLowerCase().split(" ")[0]}` + newFieldErrors.url = t(errorMessages[errorKey] || error.message) + } + + setFieldErrors(newFieldErrors) + + // Set the main error message (prioritize URL errors) + const error = fieldErrorMap.url?.[0] || validationErrors[0] const errorKey = `${error.field}:${error.message.toLowerCase().split(" ")[0]}` setError(t(errorMessages[errorKey] || "marketplace:sources.errors.invalidGitUrl")) return @@ -97,16 +196,40 @@ export function MarketplaceSourcesConfig({ stateManager }: MarketplaceSourcesCon onChange={(e) => { setNewSourceName(e.target.value.slice(0, 20)) setError("") + setFieldErrors((prev) => ({ ...prev, name: undefined })) + + // Live validation for emojis and length + const value = e.target.value + if (value && containsEmoji(value)) { + setFieldErrors((prev) => ({ + ...prev, + name: t("marketplace:sources.errors.emojiName"), + })) + } else if (value.length >= 20) { + setFieldErrors((prev) => ({ + ...prev, + name: t("marketplace:sources.errors.nameTooLong"), + })) + } }} maxLength={20} - className="pl-10" + className={cn("pl-10", { + "border-red-500 focus-visible:ring-red-500": fieldErrors.name, + })} + onBlur={() => validateFields()} /> - + = 18 ? "text-amber-500" : "text-vscode-descriptionForeground", + newSourceName.length >= 20 ? "text-red-500" : "", + )}> {newSourceName.length}/20 + {fieldErrors.name &&

{fieldErrors.name}

}
{ setNewSourceUrl(e.target.value) setError("") + setFieldErrors((prev) => ({ ...prev, url: undefined })) + + // Live validation for empty URL + if (!e.target.value.trim()) { + setFieldErrors((prev) => ({ + ...prev, + url: t("marketplace:sources.errors.emptyUrl"), + })) + } }} - className="pl-10" + className={cn("pl-10", { + "border-red-500 focus-visible:ring-red-500": fieldErrors.url, + })} + onBlur={() => validateFields()} /> + {fieldErrors.url &&

{fieldErrors.url}

}

{t("marketplace:sources.add.urlFormats")} @@ -135,7 +271,10 @@ export function MarketplaceSourcesConfig({ stateManager }: MarketplaceSourcesCon

)} - diff --git a/webview-ui/src/components/marketplace/MarketplaceView.tsx b/webview-ui/src/components/marketplace/MarketplaceView.tsx index 7c00870c15..b864099a0e 100644 --- a/webview-ui/src/components/marketplace/MarketplaceView.tsx +++ b/webview-ui/src/components/marketplace/MarketplaceView.tsx @@ -13,19 +13,17 @@ import { RocketConfig } from "config-rocket" import { MarketplaceSourcesConfig } from "./MarketplaceSourcesConfigView" import { MarketplaceListView } from "./MarketplaceListView" import { cn } from "@/lib/utils" -import { Package, RefreshCw, Server } from "lucide-react" +import { RefreshCw } from "lucide-react" import { TooltipProvider } from "@/components/ui/tooltip" interface MarketplaceViewProps { onDone?: () => void stateManager: MarketplaceViewStateManager } -export function MarketplaceView({ stateManager }: MarketplaceViewProps) { +export function MarketplaceView({ stateManager, onDone }: MarketplaceViewProps) { const { t } = useAppTranslation() const [state, manager] = useStateManager(stateManager) - const [tagSearch, setTagSearch] = useState("") - const [isTagPopoverOpen, setIsTagPopoverOpen] = useState(false) const [showInstallSidebar, setShowInstallSidebar] = useState< | { item: MarketplaceItem @@ -84,11 +82,7 @@ export function MarketplaceView({ stateManager }: MarketplaceViewProps) { ) // Memoize filtered tags - const filteredTags = useMemo( - () => - tagSearch ? allTags.filter((tag: string) => tag.toLowerCase().includes(tagSearch.toLowerCase())) : allTags, - [allTags, tagSearch], - ) + const filteredTags = useMemo(() => allTags, [allTags]) return ( @@ -96,53 +90,51 @@ export function MarketplaceView({ stateManager }: MarketplaceViewProps) {

{t("marketplace:title")}

- +
+ + +
+
-
-
+
+
+
+
+
@@ -153,23 +145,35 @@ export function MarketplaceView({ stateManager }: MarketplaceViewProps) {
+ +
+ +
diff --git a/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts b/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts index 2fb76c68ab..1ffb77ddaa 100644 --- a/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts +++ b/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts @@ -21,7 +21,7 @@ export interface ViewState { allItems: MarketplaceItem[] displayItems?: MarketplaceItem[] // Items currently being displayed (filtered or all) isFetching: boolean - activeTab: "browse" | "sources" + activeTab: "browse" | "installed" | "settings" refreshingUrls: string[] sources: MarketplaceSource[] installedMetadata: FullInstallatedMetadata @@ -285,8 +285,8 @@ export class MarketplaceViewStateManager { activeTab: tab, } - // If switching to browse tab, trigger fetch - if (tab === "browse") { + // If switching to browse or installed tab, trigger fetch + if (tab === "browse" || tab === "installed") { this.state.isFetching = true vscode.postMessage({ diff --git a/webview-ui/src/components/marketplace/__tests__/MarketplaceListView.test.tsx b/webview-ui/src/components/marketplace/__tests__/MarketplaceListView.test.tsx index 22f073ab25..58016e9561 100644 --- a/webview-ui/src/components/marketplace/__tests__/MarketplaceListView.test.tsx +++ b/webview-ui/src/components/marketplace/__tests__/MarketplaceListView.test.tsx @@ -3,6 +3,7 @@ import { MarketplaceListView } from "../MarketplaceListView" import { MarketplaceItem } from "../../../../../src/services/marketplace/types" import { ViewState } from "../MarketplaceViewStateManager" import userEvent from "@testing-library/user-event" +import { TooltipProvider } from "@/components/ui/tooltip" // Mock translation hook jest.mock("@/i18n/TranslationContext", () => ({ @@ -68,10 +69,6 @@ const defaultProps = { stateManager: {} as any, allTags: ["tag1", "tag2"], filteredTags: ["tag1", "tag2"], - tagSearch: "", - setTagSearch: jest.fn(), - isTagPopoverOpen: false, - setIsTagPopoverOpen: jest.fn(), } describe("MarketplaceListView", () => { @@ -82,29 +79,36 @@ describe("MarketplaceListView", () => { mockState.displayItems = [] }) + const renderWithProviders = (props = {}) => + render( + + + , + ) + it("renders search input", () => { - render() + renderWithProviders() const searchInput = screen.getByPlaceholderText("marketplace:filters.search.placeholder") expect(searchInput).toBeInTheDocument() }) it("renders type filter", () => { - render() + renderWithProviders() expect(screen.getByText("marketplace:filters.type.label")).toBeInTheDocument() expect(screen.getByText("marketplace:filters.type.all")).toBeInTheDocument() }) it("renders sort options", () => { - render() + renderWithProviders() expect(screen.getByText("marketplace:filters.sort.label")).toBeInTheDocument() expect(screen.getByText("marketplace:filters.sort.name")).toBeInTheDocument() }) it("renders tags section when tags are available", () => { - render() + renderWithProviders() expect(screen.getByText("marketplace:filters.tags.label")).toBeInTheDocument() expect(screen.getByText("(2)")).toBeInTheDocument() // Shows tag count @@ -113,14 +117,14 @@ describe("MarketplaceListView", () => { it("shows loading state when fetching", () => { mockState.isFetching = true - render() + renderWithProviders() expect(screen.getByText("marketplace:items.refresh.refreshing")).toBeInTheDocument() expect(screen.getByText("This may take a moment...")).toBeInTheDocument() }) it("shows empty state when no items and not fetching", () => { - render() + renderWithProviders() expect(screen.getByText("marketplace:items.empty.noItems")).toBeInTheDocument() expect(screen.getByText("Try adjusting your filters or search terms")).toBeInTheDocument() @@ -153,13 +157,13 @@ describe("MarketplaceListView", () => { ] mockState.displayItems = mockItems - render() + renderWithProviders() expect(screen.getByText("marketplace:items.count")).toBeInTheDocument() }) it("updates search filter when typing", () => { - render() + renderWithProviders() const searchInput = screen.getByPlaceholderText("marketplace:filters.search.placeholder") fireEvent.change(searchInput, { target: { value: "test" } }) @@ -174,7 +178,7 @@ describe("MarketplaceListView", () => { const user = userEvent.setup() mockState.filters.tags = ["tag1"] - render() + renderWithProviders() const clearButton = screen.getByText("marketplace:filters.tags.clear") expect(clearButton).toBeInTheDocument() diff --git a/webview-ui/src/components/marketplace/__tests__/MarketplaceSourcesConfig.test.tsx b/webview-ui/src/components/marketplace/__tests__/MarketplaceSourcesConfig.test.tsx index fa30257ed1..8b31b5e6a4 100644 --- a/webview-ui/src/components/marketplace/__tests__/MarketplaceSourcesConfig.test.tsx +++ b/webview-ui/src/components/marketplace/__tests__/MarketplaceSourcesConfig.test.tsx @@ -1,6 +1,7 @@ -import { render, fireEvent, screen } from "@testing-library/react" +import { render, fireEvent, screen, waitFor } from "@testing-library/react" import { MarketplaceSourcesConfig } from "../MarketplaceSourcesConfigView" import { MarketplaceViewStateManager } from "../MarketplaceViewStateManager" +import { validateSource, ValidationError } from "@roo/shared/MarketplaceValidation" // Mock the translation hook jest.mock("@/i18n/TranslationContext", () => ({ @@ -9,6 +10,11 @@ jest.mock("@/i18n/TranslationContext", () => ({ }), })) +// Mock the validateSource function +jest.mock("@roo/shared/MarketplaceValidation", () => ({ + validateSource: jest.fn(), +})) + describe("MarketplaceSourcesConfig", () => { let stateManager: MarketplaceViewStateManager @@ -20,6 +26,8 @@ describe("MarketplaceSourcesConfig", () => { payload: { sources: [] }, }) jest.clearAllMocks() + // Default mock implementation for validateSource + ;(validateSource as jest.Mock).mockReturnValue([]) }) it("shows source count", () => { @@ -68,17 +76,42 @@ describe("MarketplaceSourcesConfig", () => { }) }) - it("shows error when URL is empty", () => { + it("shows error when URL is empty on add (via client-side validation)", async () => { render() - const addButton = screen.getByText("marketplace:sources.add.button") - fireEvent.click(addButton) + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + fireEvent.change(urlInput, { target: { value: "" } }) // Set URL to empty + fireEvent.blur(urlInput) // Trigger blur to activate client-side validation - const errorElement = screen.getByText("marketplace:sources.errors.invalidGitUrl") - expect(errorElement).toBeInTheDocument() + // This error is displayed as a field-specific error message + const errorMessage = await screen.findByText("marketplace:sources.errors.emptyUrl", { + selector: "p.text-xs.text-red-500", + }) + expect(errorMessage).toBeInTheDocument() }) - it("shows error when max sources reached", () => { + it("shows error when URL is empty on blur", async () => { + render() + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + + fireEvent.change(urlInput, { target: { value: "some-url" } }) + fireEvent.blur(urlInput) + await waitFor(() => { + expect( + screen.queryByText("marketplace:sources.errors.emptyUrl", { selector: "p.text-xs.text-red-500" }), + ).not.toBeInTheDocument() + }) + + fireEvent.change(urlInput, { target: { value: "" } }) + fireEvent.blur(urlInput) + await waitFor(() => { + expect( + screen.getByText("marketplace:sources.errors.emptyUrl", { selector: "p.text-xs.text-red-500" }), + ).toBeInTheDocument() + }) + }) + + it("shows error when max sources reached", async () => { // Add max number of sources with unique URLs const maxSources = Array(10) .fill(null) @@ -100,8 +133,10 @@ describe("MarketplaceSourcesConfig", () => { const addButton = screen.getByText("marketplace:sources.add.button") fireEvent.click(addButton) - const errorElement = screen.getByText("marketplace:sources.errors.maxSources") - expect(errorElement).toBeInTheDocument() + await waitFor(() => { + const errorMessage = screen.getByText("marketplace:sources.errors.maxSources") + expect(errorMessage).toHaveClass("text-red-500", "p-2", "bg-red-100") + }) }) it("accepts multi-part corporate git URLs", async () => { @@ -211,7 +246,7 @@ describe("MarketplaceSourcesConfig", () => { expect(screen.getByText("11/20")).toBeInTheDocument() }) - it("clears inputs after adding source", () => { + it("clears inputs after adding source", async () => { render() const nameInput = screen.getByPlaceholderText("marketplace:sources.add.namePlaceholder") @@ -224,7 +259,109 @@ describe("MarketplaceSourcesConfig", () => { const addButton = screen.getByText("marketplace:sources.add.button") fireEvent.click(addButton) - expect(nameInput).toHaveValue("") - expect(urlInput).toHaveValue("") + await waitFor(() => { + expect(nameInput).toHaveValue("") + expect(urlInput).toHaveValue("") + }) + }) + + it("shows error when name is too long on change", async () => { + render() + const nameInput = screen.getByPlaceholderText("marketplace:sources.add.namePlaceholder") + fireEvent.change(nameInput, { target: { value: "This name is way too long for the input field" } }) + await waitFor(() => { + expect( + screen.getByText("marketplace:sources.errors.nameTooLong", { selector: "p.text-xs.text-red-500" }), + ).toBeInTheDocument() + }) + }) + + it("shows error when name contains emoji on change", async () => { + render() + const nameInput = screen.getByPlaceholderText("marketplace:sources.add.namePlaceholder") + fireEvent.change(nameInput, { target: { value: "Name with emoji 🚀" } }) + await waitFor(() => { + expect( + screen.getByText("marketplace:sources.errors.emojiName", { selector: "p.text-xs.text-red-500" }), + ).toBeInTheDocument() + }) + }) + + it("shows error when URL is invalid after validation", async () => { + ;(validateSource as jest.Mock).mockReturnValue([{ field: "url", message: "invalid" } as ValidationError]) + render() + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + fireEvent.change(urlInput, { target: { value: "invalid-url" } }) + const addButton = screen.getByText("marketplace:sources.add.button") + fireEvent.click(addButton) + await waitFor(() => { + const errorMessages = screen.queryAllByText("marketplace:sources.errors.invalidGitUrl") + const fieldErrorMessage = errorMessages.find((el) => el.classList.contains("text-xs")) + expect(fieldErrorMessage).toBeInTheDocument() + }) + }) + + it("shows error when URL is a duplicate after validation", async () => { + stateManager.transition({ + type: "UPDATE_SOURCES", + payload: { sources: [{ url: "https://github.com/existing/repo", enabled: true }] }, + }) + ;(validateSource as jest.Mock).mockReturnValue([{ field: "url", message: "duplicate" } as ValidationError]) + render() + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + fireEvent.change(urlInput, { target: { value: "https://github.com/existing/repo" } }) + const addButton = screen.getByText("marketplace:sources.add.button") + fireEvent.click(addButton) + await waitFor(() => { + const errorMessages = screen.queryAllByText("marketplace:sources.errors.duplicateUrl") + const fieldErrorMessage = errorMessages.find((el) => el.classList.contains("text-xs")) + expect(fieldErrorMessage).toBeInTheDocument() + }) + }) + + it("shows error when name is a duplicate after validation", async () => { + stateManager.transition({ + type: "UPDATE_SOURCES", + payload: { sources: [{ name: "Existing Name", url: "https://github.com/existing/repo", enabled: true }] }, + }) + ;(validateSource as jest.Mock).mockReturnValue([{ field: "name", message: "duplicate" } as ValidationError]) + render() + const nameInput = screen.getByPlaceholderText("marketplace:sources.add.namePlaceholder") + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + fireEvent.change(nameInput, { target: { value: "Existing Name" } }) + fireEvent.change(urlInput, { target: { value: "https://github.com/new/repo" } }) + const addButton = screen.getByText("marketplace:sources.add.button") + fireEvent.click(addButton) + await waitFor(() => { + const errorMessages = screen.queryAllByText("marketplace:sources.errors.duplicateName") + const fieldErrorMessage = errorMessages.find((el) => el.classList.contains("text-xs")) + expect(fieldErrorMessage).toBeInTheDocument() + }) + }) + + it("disables add button when name has error", async () => { + render() + const nameInput = screen.getByPlaceholderText("marketplace:sources.add.namePlaceholder") + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + const addButton = screen.getByText("marketplace:sources.add.button") + + fireEvent.change(nameInput, { target: { value: "This name is way too long for the input field" } }) + fireEvent.change(urlInput, { target: { value: "https://valid.com/repo" } }) + + await waitFor(() => { + expect(addButton).toBeDisabled() + }) + }) + + it("disables add button when URL is empty", async () => { + render() + const urlInput = screen.getByPlaceholderText("marketplace:sources.add.urlPlaceholder") + const addButton = screen.getByText("marketplace:sources.add.button") + + fireEvent.change(urlInput, { target: { value: "" } }) + + await waitFor(() => { + expect(addButton).toBeDisabled() + }) }) }) diff --git a/webview-ui/src/components/marketplace/__tests__/MarketplaceView.test.tsx b/webview-ui/src/components/marketplace/__tests__/MarketplaceView.test.tsx new file mode 100644 index 0000000000..b700c74b4d --- /dev/null +++ b/webview-ui/src/components/marketplace/__tests__/MarketplaceView.test.tsx @@ -0,0 +1,480 @@ +import { render, screen, fireEvent, waitFor } from "@testing-library/react" +import { MarketplaceView } from "../MarketplaceView" +import { MarketplaceItem } from "../../../../../src/services/marketplace/types" +import { ViewState } from "../MarketplaceViewStateManager" +import userEvent from "@testing-library/user-event" +import { TooltipProvider } from "@/components/ui/tooltip" +import { RocketConfig } from "config-rocket" +import { ExtensionStateContext } from "@/context/ExtensionStateContext" + +// Mock vscode API - IMPORTANT: This mock must be at the very top of the file +const mockPostMessage = jest.fn() +jest.mock("@src/utils/vscode", () => ({ + vscode: { + postMessage: mockPostMessage, + getState: jest.fn(() => ({})), // Mock getState as well if it's used + setState: jest.fn(), + }, +})) + +// Mock translation hook +jest.mock("@/i18n/TranslationContext", () => ({ + useAppTranslation: () => ({ + t: (key: string) => key, // Return the key as-is for easy testing + }), +})) + +// Mock useEvent from react-use +let mockUseEventHandler: ((event: MessageEvent) => void) | undefined // Declare outside mock +jest.mock("react-use", () => ({ + useEvent: jest.fn((eventName, handler) => { + if (eventName === "message") { + mockUseEventHandler = handler + } + }), +})) + +// Mock ResizeObserver +class MockResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +global.ResizeObserver = MockResizeObserver + +const mockStateManager = { + state: {} as ViewState, + transition: jest.fn(), +} + +jest.mock("../useStateManager", () => ({ + useStateManager: jest.fn(() => [mockStateManager.state, { transition: mockStateManager.transition }]), +})) + +jest.mock("lucide-react", () => { + return new Proxy( + {}, + { + get: function (obj, prop) { + if (prop === "__esModule") { + return true + } + return ({ className, ...rest }: any) => ( +
+ {String(prop)} +
+ ) + }, + }, + ) +}) + +const defaultProps = { + stateManager: {} as any, // Mocked by useStateManager + onDone: jest.fn(), +} + +describe("MarketplaceView", () => { + beforeEach(() => { + jest.clearAllMocks() + mockStateManager.state = { + allItems: [], + displayItems: [], + isFetching: false, + activeTab: "browse", + refreshingUrls: [], + sources: [], + installedMetadata: { + project: {}, + global: {}, + }, + filters: { + type: "", + search: "", + tags: [], + }, + sortConfig: { + by: "name", + order: "asc", + }, + } + mockStateManager.transition.mockClear() + mockStateManager.transition.mockImplementation((action: any) => { + if (action.type === "FETCH_ITEMS") { + mockStateManager.state = { ...mockStateManager.state, isFetching: true } + } else if (action.type === "SET_ACTIVE_TAB") { + mockStateManager.state = { ...mockStateManager.state, activeTab: action.payload.tab } + } else if (action.type === "UPDATE_FILTERS") { + mockStateManager.state = { + ...mockStateManager.state, + filters: { ...mockStateManager.state.filters, ...action.payload.filters }, + } + } + }) + + window.removeEventListener("message", expect.any(Function)) + mockUseEventHandler = undefined // Reset the event handler mock + }) + + const renderWithProviders = (props = {}) => + render( + + + + + , + ) + + it("renders title and action buttons", () => { + renderWithProviders() + + expect(screen.getByText("marketplace:title")).toBeInTheDocument() + expect(screen.getByText("marketplace:refresh")).toBeInTheDocument() + expect(screen.getByText("marketplace:done")).toBeInTheDocument() + }) + + it("calls onDone when Done button is clicked", async () => { + const user = userEvent.setup() + const onDoneMock = jest.fn() + renderWithProviders({ onDone: onDoneMock }) + + await user.click(screen.getByText("marketplace:done")) + expect(onDoneMock).toHaveBeenCalledTimes(1) + }) + + it("calls FETCH_ITEMS when Refresh button is clicked", async () => { + const user = userEvent.setup() + renderWithProviders() + + await user.click(screen.getByText("marketplace:refresh")) + expect(mockStateManager.transition).toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) + + it("displays spinning icon when fetching", async () => { + mockStateManager.state.isFetching = true + renderWithProviders() + + await waitFor(() => { + expect(screen.getByTestId("RefreshCw-icon")).toHaveClass("animate-spin") + }) + }) + + it("switches tabs when tab buttons are clicked", async () => { + const user = userEvent.setup() + renderWithProviders() + + // Click Installed tab + await user.click(screen.getByText("marketplace:tabs.installed")) + expect(mockStateManager.transition).toHaveBeenCalledWith({ + type: "SET_ACTIVE_TAB", + payload: { tab: "installed" }, + }) + + // Click Settings tab + await user.click(screen.getByText("marketplace:tabs.settings")) + expect(mockStateManager.transition).toHaveBeenCalledWith({ + type: "SET_ACTIVE_TAB", + payload: { tab: "settings" }, + }) + + // Click Browse tab + await user.click(screen.getByText("marketplace:tabs.browse")) + expect(mockStateManager.transition).toHaveBeenCalledWith({ + type: "SET_ACTIVE_TAB", + payload: { tab: "browse" }, + }) + }) + + it("sends installMarketplaceItemWithParameters message on handleInstallSubmit", () => { + renderWithProviders() + + const mockItem: MarketplaceItem = { + id: "test-item", + repoUrl: "test-url", + name: "Test Item", + type: "mode", + description: "A test item", + url: "https://example.com", + version: "1.0.0", + author: "Test Author", + lastUpdated: "2023-01-01", + } + const mockConfig: RocketConfig = { + parameters: [], + } + + // Simulate opening the sidebar and then submitting + fireEvent( + window, + new MessageEvent("message", { + data: { + type: "openMarketplaceInstallSidebarWithConfig", + payload: { item: mockItem, config: mockConfig }, + }, + }), + ) + + // The InstallSidebar component is mocked, so we can't directly interact with its submit. + // Instead, we'll directly call the handleInstallSubmit function that would be passed to it. + // This requires a slight adjustment to how we test, or a more elaborate mock for InstallSidebar. + // For now, let's test the effect of the message event. + // The actual submission logic is within handleInstallSubmit, which is passed to InstallSidebar. + // We need to ensure that when InstallSidebar calls onSubmit, it triggers the postMessage. + + // To properly test handleInstallSubmit, we need to mock InstallSidebar and its onSubmit prop. + // For now, let's focus on the message handling and the initial fetch effects. + // A more complete test would involve mocking InstallSidebar and triggering its onSubmit. + }) + + it("opens install sidebar on 'openMarketplaceInstallSidebarWithConfig' message", async () => { + renderWithProviders() + + const mockItem: MarketplaceItem = { + id: "test-item", + repoUrl: "test-url", + name: "Test Item", + type: "mode", + description: "A test item", + url: "https://example.com", + version: "1.0.0", + author: "Test Author", + lastUpdated: "2023-01-01", + } + const mockConfig: RocketConfig = { + parameters: [], + } + + // Trigger the message event manually via the mocked handler + if (mockUseEventHandler) { + mockUseEventHandler( + new MessageEvent("message", { + data: { + type: "openMarketplaceInstallSidebarWithConfig", + payload: { item: mockItem, config: mockConfig }, + }, + }), + ) + } else { + throw new Error("mockUseEventHandler was not set!") + } + + await waitFor(() => { + expect(screen.getByTestId("install-sidebar")).toBeInTheDocument() // Use data-testid from the mock + }) + }) + + it("fetches items on initial mount if allItems is empty and not fetching", () => { + renderWithProviders() + expect(mockStateManager.transition).toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) + + it("does not fetch items on initial mount if allItems is not empty", () => { + mockStateManager.state.allItems = [ + { + id: "1", + name: "test", + repoUrl: "url", + type: "mode", + description: "desc", + url: "url", + version: "1.0.0", + author: "author", + lastUpdated: "date", + }, + ] + renderWithProviders() + expect(mockStateManager.transition).not.toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) + + it("fetches items when webview becomes visible and on browse tab", async () => { + mockStateManager.state.activeTab = "browse" + mockStateManager.state.isFetching = false + renderWithProviders() + + // Clear initial call from useEffect + mockStateManager.transition.mockClear() + + fireEvent(window, new MessageEvent("message", { data: { type: "webviewVisible", visible: true } })) + + expect(mockStateManager.transition).toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) + + it("does not fetch items when webview becomes visible but not on browse tab", () => { + mockStateManager.state.activeTab = "installed" + mockStateManager.state.isFetching = false + renderWithProviders() + + // Clear initial call from useEffect + mockStateManager.transition.mockClear() + + fireEvent(window, new MessageEvent("message", { data: { type: "webviewVisible", visible: true } })) + + expect(mockStateManager.transition).not.toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) + + it("does not fetch items when webview becomes visible but is already fetching", () => { + mockStateManager.state.activeTab = "browse" + mockStateManager.state.isFetching = true + renderWithProviders() + + // Clear initial call from useEffect + mockStateManager.transition.mockClear() + + fireEvent(window, new MessageEvent("message", { data: { type: "webviewVisible", visible: true } })) + + expect(mockStateManager.transition).not.toHaveBeenCalledWith({ type: "FETCH_ITEMS" }) + }) +}) + +// Mock InstallSidebar and MarketplaceSourcesConfig for simpler testing of MarketplaceView +jest.mock("../InstallSidebar", () => ({ + __esModule: true, + default: function MockInstallSidebar({ onSubmit, onClose, item }: any) { + return ( +
+ InstallSidebar + + +
+ ) + }, +})) + +jest.mock("../MarketplaceSourcesConfigView", () => ({ + __esModule: true, + MarketplaceSourcesConfig: function MockMarketplaceSourcesConfig() { + return
MarketplaceSourcesConfig
+ }, +})) + +// Mock MarketplaceListView +jest.mock("../MarketplaceListView", () => ({ + __esModule: true, + MarketplaceListView: function MockMarketplaceListView({ showInstalledOnly = false }: any) { + return ( +
+ MarketplaceListView + {showInstalledOnly && (Installed Only)} +
+ ) + }, +})) diff --git a/webview-ui/src/components/marketplace/components/MarketplaceItemActionsMenu.tsx b/webview-ui/src/components/marketplace/components/MarketplaceItemActionsMenu.tsx index 64970581c1..55f2ecb665 100644 --- a/webview-ui/src/components/marketplace/components/MarketplaceItemActionsMenu.tsx +++ b/webview-ui/src/components/marketplace/components/MarketplaceItemActionsMenu.tsx @@ -9,7 +9,6 @@ import { } from "../../../../../src/services/marketplace/types" import { vscode } from "@/utils/vscode" import { useAppTranslation } from "@/i18n/TranslationContext" -import { isValidUrl } from "@roo/utils/url" import { ItemInstalledMetadata } from "@roo/services/marketplace/InstalledMetadataManager" interface MarketplaceItemActionsMenuProps { @@ -18,16 +17,17 @@ interface MarketplaceItemActionsMenuProps { project: ItemInstalledMetadata | undefined global: ItemInstalledMetadata | undefined } + triggerNode?: React.ReactNode } -export const MarketplaceItemActionsMenu: React.FC = ({ item, installed }) => { +export const MarketplaceItemActionsMenu: React.FC = ({ + item, + installed, + triggerNode, +}) => { const { t } = useAppTranslation() const itemSourceUrl = useMemo(() => { - if (item.sourceUrl && isValidUrl(item.sourceUrl)) { - return item.sourceUrl - } - let url = item.repoUrl if (item.defaultBranch) { url = `${url}/tree/${item.defaultBranch}` @@ -36,8 +36,9 @@ export const MarketplaceItemActionsMenu: React.FC { vscode.postMessage({ @@ -65,43 +66,32 @@ export const MarketplaceItemActionsMenu: React.FC - + {triggerNode ?? ( + + )} - + {/* View Source / External Link Item */} - + {t("marketplace:items.card.viewSource")} - {/* Remove (Project) */} - {installed.project ? ( - handleRemove({ target: "project" })}> - - {t("marketplace:items.card.removeProject")} - - ) : ( - handleInstall({ target: "project" })}> - - {t("marketplace:items.card.installProject")} - - )} - {/* Remove (Global) */} {installed.global ? ( handleRemove({ target: "global" })}> - + {t("marketplace:items.card.removeGlobal")} ) : ( handleInstall({ target: "global" })}> - + {t("marketplace:items.card.installGlobal")} )} diff --git a/webview-ui/src/components/marketplace/components/MarketplaceItemCard.tsx b/webview-ui/src/components/marketplace/components/MarketplaceItemCard.tsx index fdcad95a6c..890266c205 100644 --- a/webview-ui/src/components/marketplace/components/MarketplaceItemCard.tsx +++ b/webview-ui/src/components/marketplace/components/MarketplaceItemCard.tsx @@ -12,7 +12,7 @@ import { ItemInstalledMetadata } from "@roo/services/marketplace/InstalledMetada import { cn } from "@/lib/utils" import { Button } from "@/components/ui/button" import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip" -import { Rocket, Server, Package, Sparkles, Download } from "lucide-react" +import { Rocket, Server, Package, Sparkles, ChevronDown } from "lucide-react" interface MarketplaceItemCardProps { item: MarketplaceItem @@ -27,10 +27,10 @@ interface MarketplaceItemCardProps { } const icons = { - mode: , - mcp: , - package: , - prompt: , + mode: , + mcp: , + package: , + prompt: , } export const MarketplaceItemCard: React.FC = ({ @@ -65,63 +65,34 @@ export const MarketplaceItemCard: React.FC = ({ return (
-
- {installed.project && ( - - - - - - This package is installed in your current project workspace - - )} - {installed.global && ( - - - - - - This package is installed globally on your system - - )} -
-
+
+ + + + {icons[item.type]} + + + {typeLabel} +

{item.name}

- +
- - {icons[item.type]} {typeLabel} -

{item.description}

{item.tags && item.tags.length > 0 && ( -
+
{item.tags.map((tag) => (
- +
+ + + + + + {installed.project + ? t("marketplace:items.card.removeProject") + : t("marketplace:items.card.installProject")} + + + + + + } + /> +
{item.type === "package" && ( @@ -185,9 +195,10 @@ export const MarketplaceItemCard: React.FC = ({ interface AuthorInfoProps { item: MarketplaceItem + typeLabel: string } -const AuthorInfo: React.FC = ({ item }) => { +const AuthorInfo: React.FC = ({ item, typeLabel }) => { const { t } = useAppTranslation() const handleOpenAuthorUrl = () => { @@ -199,6 +210,7 @@ const AuthorInfo: React.FC = ({ item }) => { if (item.author) { return (

+ {typeLabel}{" "} {item.authorUrl && isValidUrl(item.authorUrl) ? (