-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add "Installed" filter to Roo Marketplace #7007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
5324f65
07c2478
9231aa5
5db2d9b
f0d881a
048b86e
d4cd952
0bf4e0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -27,6 +27,17 @@ export function MarketplaceListView({ stateManager, allTags, filteredTags, filte | |||||||||||||||
const allItems = state.displayItems || [] | ||||||||||||||||
const organizationMcps = state.displayOrganizationMcps || [] | ||||||||||||||||
|
||||||||||||||||
// Update state manager with installed metadata when it changes | ||||||||||||||||
React.useEffect(() => { | ||||||||||||||||
if (marketplaceInstalledMetadata && state.installedMetadata !== marketplaceInstalledMetadata) { | ||||||||||||||||
// Update the state manager's installed metadata | ||||||||||||||||
manager.transition({ | ||||||||||||||||
type: "UPDATE_FILTERS", | ||||||||||||||||
payload: { filters: state.filters }, | ||||||||||||||||
}) | ||||||||||||||||
} | ||||||||||||||||
}, [marketplaceInstalledMetadata, state.installedMetadata, state.filters, manager]) | ||||||||||||||||
|
||||||||||||||||
// Filter items by type if specified | ||||||||||||||||
const items = filterByType ? allItems.filter((item) => item.type === filterByType) : allItems | ||||||||||||||||
const orgMcps = filterByType === "mcp" ? organizationMcps : [] | ||||||||||||||||
|
@@ -55,6 +66,28 @@ export function MarketplaceListView({ stateManager, allTags, filteredTags, filte | |||||||||||||||
} | ||||||||||||||||
/> | ||||||||||||||||
</div> | ||||||||||||||||
{/* Installed filter toggle */} | ||||||||||||||||
<div className="mt-2 flex items-center gap-2"> | ||||||||||||||||
<label className="flex items-center gap-2 cursor-pointer"> | ||||||||||||||||
<input | ||||||||||||||||
type="checkbox" | ||||||||||||||||
className="rounded border-vscode-input-border" | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add proper ARIA labels here for better accessibility? Screen reader users might have difficulty understanding the purpose of this checkbox without proper labeling.
Suggested change
|
||||||||||||||||
checked={state.filters.installed} | ||||||||||||||||
onChange={(e) => | ||||||||||||||||
manager.transition({ | ||||||||||||||||
type: "UPDATE_FILTERS", | ||||||||||||||||
payload: { filters: { installed: e.target.checked } }, | ||||||||||||||||
}) | ||||||||||||||||
} | ||||||||||||||||
/> | ||||||||||||||||
<span className="text-sm">{t("marketplace:filters.installed.label")}</span> | ||||||||||||||||
</label> | ||||||||||||||||
{state.filters.installed && ( | ||||||||||||||||
<span className="text-xs text-vscode-descriptionForeground"> | ||||||||||||||||
({t("marketplace:filters.installed.description")}) | ||||||||||||||||
</span> | ||||||||||||||||
)} | ||||||||||||||||
</div> | ||||||||||||||||
{allTags.length > 0 && ( | ||||||||||||||||
<div className="mt-2"> | ||||||||||||||||
<div className="flex items-center justify-between mb-1"> | ||||||||||||||||
|
@@ -187,7 +220,7 @@ export function MarketplaceListView({ stateManager, allTags, filteredTags, filte | |||||||||||||||
onClick={() => | ||||||||||||||||
manager.transition({ | ||||||||||||||||
type: "UPDATE_FILTERS", | ||||||||||||||||
payload: { filters: { search: "", type: "", tags: [] } }, | ||||||||||||||||
payload: { filters: { search: "", type: "", tags: [], installed: false } }, | ||||||||||||||||
}) | ||||||||||||||||
} | ||||||||||||||||
className="mt-4 bg-vscode-button-secondaryBackground text-vscode-button-secondaryForeground hover:bg-vscode-button-secondaryHoverBackground transition-colors"> | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -26,7 +26,9 @@ export interface ViewState { | |||||||||||
type: string | ||||||||||||
search: string | ||||||||||||
tags: string[] | ||||||||||||
installed: boolean // New filter to show only installed items | ||||||||||||
} | ||||||||||||
installedMetadata?: any // Store installed metadata for filtering | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type safety concern: Consider creating a proper TypeScript interface for the installed metadata structure instead of using
Suggested change
|
||||||||||||
} | ||||||||||||
|
||||||||||||
type TransitionPayloads = { | ||||||||||||
|
@@ -65,6 +67,7 @@ export class MarketplaceViewStateManager { | |||||||||||
type: "", | ||||||||||||
search: "", | ||||||||||||
tags: [], | ||||||||||||
installed: false, | ||||||||||||
}, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||
} | ||||||||||||
} | ||||||||||||
|
@@ -189,8 +192,11 @@ export class MarketplaceViewStateManager { | |||||||||||
let newDisplayItems: MarketplaceItem[] | ||||||||||||
let newDisplayOrganizationMcps: MarketplaceItem[] | ||||||||||||
if (this.isFilterActive()) { | ||||||||||||
newDisplayItems = this.filterItems([...items]) | ||||||||||||
newDisplayOrganizationMcps = this.filterItems([...this.state.organizationMcps]) | ||||||||||||
newDisplayItems = this.filterItems([...items], this.state.installedMetadata) | ||||||||||||
newDisplayOrganizationMcps = this.filterItems( | ||||||||||||
[...this.state.organizationMcps], | ||||||||||||
this.state.installedMetadata, | ||||||||||||
) | ||||||||||||
} else { | ||||||||||||
// No filters active - show all items | ||||||||||||
newDisplayItems = [...items] | ||||||||||||
|
@@ -251,6 +257,7 @@ export class MarketplaceViewStateManager { | |||||||||||
type: filters.type !== undefined ? filters.type : this.state.filters.type, | ||||||||||||
search: filters.search !== undefined ? filters.search : this.state.filters.search, | ||||||||||||
tags: filters.tags !== undefined ? filters.tags : this.state.filters.tags, | ||||||||||||
installed: filters.installed !== undefined ? filters.installed : this.state.filters.installed, | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Update filters first | ||||||||||||
|
@@ -260,8 +267,11 @@ export class MarketplaceViewStateManager { | |||||||||||
} | ||||||||||||
|
||||||||||||
// Apply filters to displayItems and displayOrganizationMcps with the updated filters | ||||||||||||
const newDisplayItems = this.filterItems(this.state.allItems) | ||||||||||||
const newDisplayOrganizationMcps = this.filterItems(this.state.organizationMcps) | ||||||||||||
const newDisplayItems = this.filterItems(this.state.allItems, this.state.installedMetadata) | ||||||||||||
const newDisplayOrganizationMcps = this.filterItems( | ||||||||||||
this.state.organizationMcps, | ||||||||||||
this.state.installedMetadata, | ||||||||||||
) | ||||||||||||
|
||||||||||||
// Update state with filtered items | ||||||||||||
this.state = { | ||||||||||||
|
@@ -284,11 +294,16 @@ export class MarketplaceViewStateManager { | |||||||||||
} | ||||||||||||
|
||||||||||||
public isFilterActive(): boolean { | ||||||||||||
return !!(this.state.filters.type || this.state.filters.search || this.state.filters.tags.length > 0) | ||||||||||||
return !!( | ||||||||||||
this.state.filters.type || | ||||||||||||
this.state.filters.search || | ||||||||||||
this.state.filters.tags.length > 0 || | ||||||||||||
this.state.filters.installed | ||||||||||||
) | ||||||||||||
} | ||||||||||||
|
||||||||||||
public filterItems(items: MarketplaceItem[]): MarketplaceItem[] { | ||||||||||||
const { type, search, tags } = this.state.filters | ||||||||||||
public filterItems(items: MarketplaceItem[], installedMetadata?: any): MarketplaceItem[] { | ||||||||||||
const { type, search, tags, installed } = this.state.filters | ||||||||||||
|
||||||||||||
return items | ||||||||||||
.map((item) => { | ||||||||||||
|
@@ -303,9 +318,20 @@ export class MarketplaceViewStateManager { | |||||||||||
: false | ||||||||||||
const tagMatch = tags.length > 0 ? item.tags?.some((tag) => tags.includes(tag)) : false | ||||||||||||
|
||||||||||||
// Check installed status if filter is active | ||||||||||||
let installedMatch = true | ||||||||||||
if (installed && installedMetadata) { | ||||||||||||
const isInstalledGlobally = !!installedMetadata?.global?.[item.id] | ||||||||||||
const isInstalledInProject = !!installedMetadata?.project?.[item.id] | ||||||||||||
installedMatch = isInstalledGlobally || isInstalledInProject | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance consideration: The installed filter logic checks both global and project metadata for every item when the filter is active. Could we memoize the installed status calculation or compute it once when metadata updates rather than on every filter operation? This would improve performance for large item lists. |
||||||||||||
} | ||||||||||||
|
||||||||||||
// Determine if the main item matches all filters | ||||||||||||
const mainItemMatches = | ||||||||||||
typeMatch && (!search || nameMatch || descriptionMatch) && (!tags.length || tagMatch) | ||||||||||||
typeMatch && | ||||||||||||
(!search || nameMatch || descriptionMatch) && | ||||||||||||
(!tags.length || tagMatch) && | ||||||||||||
installedMatch | ||||||||||||
|
||||||||||||
const hasMatchingSubcomponents = false | ||||||||||||
|
||||||||||||
|
@@ -343,20 +369,29 @@ export class MarketplaceViewStateManager { | |||||||||||
// Handle state updates for marketplace items | ||||||||||||
// The state.marketplaceItems come from ClineProvider, see the file src/core/webview/ClineProvider.ts | ||||||||||||
const marketplaceItems = message.state.marketplaceItems | ||||||||||||
const marketplaceInstalledMetadata = message.state.marketplaceInstalledMetadata | ||||||||||||
|
||||||||||||
if (marketplaceItems !== undefined) { | ||||||||||||
// Always use the marketplace items from the extension when they're provided | ||||||||||||
// This ensures fresh data is always displayed | ||||||||||||
const items = [...marketplaceItems] | ||||||||||||
|
||||||||||||
// Update installed metadata if provided | ||||||||||||
if (marketplaceInstalledMetadata !== undefined) { | ||||||||||||
this.state.installedMetadata = marketplaceInstalledMetadata | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Calculate display items based on current filters | ||||||||||||
// If no filters are active, show all items | ||||||||||||
// If filters are active, apply filtering | ||||||||||||
let newDisplayItems: MarketplaceItem[] | ||||||||||||
let newDisplayOrganizationMcps: MarketplaceItem[] | ||||||||||||
if (this.isFilterActive()) { | ||||||||||||
newDisplayItems = this.filterItems(items) | ||||||||||||
newDisplayOrganizationMcps = this.filterItems(this.state.organizationMcps) | ||||||||||||
newDisplayItems = this.filterItems(items, this.state.installedMetadata) | ||||||||||||
newDisplayOrganizationMcps = this.filterItems( | ||||||||||||
this.state.organizationMcps, | ||||||||||||
this.state.installedMetadata, | ||||||||||||
) | ||||||||||||
} else { | ||||||||||||
// No filters active - show all items | ||||||||||||
newDisplayItems = items | ||||||||||||
|
@@ -370,6 +405,7 @@ export class MarketplaceViewStateManager { | |||||||||||
allItems: items, | ||||||||||||
displayItems: newDisplayItems, | ||||||||||||
displayOrganizationMcps: newDisplayOrganizationMcps, | ||||||||||||
installedMetadata: marketplaceInstalledMetadata || this.state.installedMetadata, | ||||||||||||
} | ||||||||||||
// Notification is handled below after all state parts are processed | ||||||||||||
} | ||||||||||||
|
@@ -411,14 +447,25 @@ export class MarketplaceViewStateManager { | |||||||||||
if (message.type === "marketplaceData") { | ||||||||||||
const marketplaceItems = message.marketplaceItems | ||||||||||||
const organizationMcps = message.organizationMcps || [] | ||||||||||||
const marketplaceInstalledMetadata = message.marketplaceInstalledMetadata | ||||||||||||
|
||||||||||||
if (marketplaceItems !== undefined) { | ||||||||||||
// Always use the marketplace items from the extension when they're provided | ||||||||||||
// This ensures fresh data is always displayed | ||||||||||||
const items = [...marketplaceItems] | ||||||||||||
const orgMcps = [...organizationMcps] | ||||||||||||
const newDisplayItems = this.isFilterActive() ? this.filterItems(items) : items | ||||||||||||
const newDisplayOrganizationMcps = this.isFilterActive() ? this.filterItems(orgMcps) : orgMcps | ||||||||||||
|
||||||||||||
// Update installed metadata if provided | ||||||||||||
if (marketplaceInstalledMetadata !== undefined) { | ||||||||||||
this.state.installedMetadata = marketplaceInstalledMetadata | ||||||||||||
} | ||||||||||||
|
||||||||||||
const newDisplayItems = this.isFilterActive() | ||||||||||||
? this.filterItems(items, this.state.installedMetadata) | ||||||||||||
: items | ||||||||||||
const newDisplayOrganizationMcps = this.isFilterActive() | ||||||||||||
? this.filterItems(orgMcps, this.state.installedMetadata) | ||||||||||||
: orgMcps | ||||||||||||
|
||||||||||||
// Update state in a single operation | ||||||||||||
this.state = { | ||||||||||||
|
@@ -428,6 +475,7 @@ export class MarketplaceViewStateManager { | |||||||||||
organizationMcps: orgMcps, | ||||||||||||
displayItems: newDisplayItems, | ||||||||||||
displayOrganizationMcps: newDisplayOrganizationMcps, | ||||||||||||
installedMetadata: marketplaceInstalledMetadata || this.state.installedMetadata, | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ const mockState: ViewState = { | |
type: "", | ||
search: "", | ||
tags: [], | ||
installed: false, | ||
}, | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this useEffect intentional? It seems redundant since the state manager already handles installedMetadata updates through the handleMessage method. This could cause unnecessary re-renders and state updates when the metadata changes.