-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: always preserve marketplace search state in URL #94
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: greptile_combined_20260121_qodo_grep_cursor_copilot_1_base_refactor_always_preserve_marketplace_search_state_in_url_pr415
Are you sure you want to change the base?
Conversation
Greptile SummaryRefactored marketplace state management to always persist search state in URL query parameters using Major changes:
Critical issues found:
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Marketplace
participant URL as URL Query Params
participant Atoms as State Hooks
participant Components
User->>Marketplace: Visit marketplace page
Marketplace->>URL: Read search params (q, tab, tags)
URL->>Atoms: useQueryState hooks initialize
Atoms->>Components: Provide state via hooks
User->>Components: Change search text
Components->>Atoms: useSearchPluginText()[1](newValue)
Atoms->>URL: Update 'q' param via nuqs
URL-->>Components: Re-render with new state
User->>Components: Switch plugin category
Components->>Atoms: useActivePluginType()[1](newCategory)
Atoms->>URL: Update 'tab' param via nuqs
Atoms->>Atoms: Check PLUGIN_CATEGORY_WITH_COLLECTIONS
alt Category has collections (all, tool)
Atoms->>Atoms: Set searchMode = null (show collections)
else Category without collections
Atoms->>Atoms: Force searchMode = true (show search results)
end
URL-->>Components: Re-render with new category
User->>Components: Add/remove filter tags
Components->>Atoms: useFilterPluginTags()[1](newTags)
Atoms->>URL: Update 'tags' param via nuqs
URL-->>Components: Re-render with filtered results
|
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.
4 files reviewed, 2 comments
| const isSearchMode = !!searchPluginText | ||
| || filterPluginTags.length > 0 | ||
| || (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) | ||
| || (searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) |
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.
logic: Logic inverted - negation operator missing. Categories WITH collections ('all', 'tool') should show collection view by default, while categories WITHOUT collections should force search mode. The original logic !PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType) was correct.
| || (searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) | |
| || (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/app/components/plugins/marketplace/atoms.ts
Line: 43:43
Comment:
**logic:** Logic inverted - negation operator missing. Categories WITH collections (`'all'`, `'tool'`) should show collection view by default, while categories WITHOUT collections should force search mode. The original logic `!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)` was correct.
```suggestion
|| (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)))
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) { | ||
| export function usePluginPageContext(selector: any) { |
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.
syntax: Using any type violates TypeScript type safety standards (AGENTS.md rule #9). The original type signature (value: PluginPageContextValue) => any should be preserved.
| export function usePluginPageContext(selector: any) { | |
| export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) { |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/app/components/plugins/plugin-page/context.tsx
Line: 51:51
Comment:
**syntax:** Using `any` type violates TypeScript type safety standards (AGENTS.md rule #9). The original type signature `(value: PluginPageContextValue) => any` should be preserved.
```suggestion
export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) {
```
**Context Used:** Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=1028e8c2-7220-42d3-94ac-c7ba65fc7240))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Benchmark PR from qodo-benchmark#415