feat(frontend/builder): add filters to blocks menu #11654
Conversation
- Updated `BlockMenuFilters` to include creator filtering functionality. - Modified `FilterChip` component to support hover animations and improved visual feedback. - Integrated `FilterSheet` for better category management. - Enhanced state management in `blockMenuStore` to handle creators and category counts. - Added animations using `framer-motion` for a smoother user experience.
github-project-automation
bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
Thank you for your contribution! Before this PR can be merged, there are several issues that need to be addressed:
Your code implementation looks good, adding filtering functionality to the blocks menu, but we need the PR metadata to be properly filled out to understand the context and ensure the changes have been properly tested. Let me know if you need any clarification or assistance with these updates! |
Abhi1992002
changed the title
|
Thank you for your contribution to add filters to the blocks menu! Your code looks well-structured and implements this feature nicely. However, before this PR can be merged, you need to complete the PR description:
If there are no configuration changes required, you can leave those checklist items as is. Once you've updated the description with these details, we can proceed with the review. |
Abhi1992002
requested review from
0ubbe and
kcze
and removed request for
a team
…ype safety - Replaced instances of GetV2BuilderSearchFilterAnyOfItem with FilterType across BlockMenuFilters, FilterSheet, and blockMenuStore for better type consistency. - Enhanced state management in useFilterSheet and useBlockMenuSearchContent to align with the new FilterType. - Updated constants and types to reflect the new filtering structure, ensuring a more robust and maintainable codebase.
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
...components/NewControlPanel/NewBlockMenu/BlockMenuSearchContent/useBlockMenuSearchContent.tsx
Show resolved
Hide resolved
- Removed the `params` option from the `customMutator` function, streamlining the request options. - Updated the construction of the full URL to directly use the provided URL without appending query parameters. - This change enhances code clarity and reduces complexity in handling request options.
…nyOfItem for improved type consistency - Replaced instances of FilterType with GetV2BuilderSearchFilterAnyOfItem across BlockMenuFilters, FilterSheet, and blockMenuStore to enhance type safety. - Adjusted state management in useFilterSheet and useBlockMenuSearchContent to align with the new filtering structure. - Updated constants and types to reflect the changes, ensuring a more robust and maintainable codebase.
| removeCreator, | ||
| } = useBlockMenuStore(); | ||
|
|
||
| const handleFilterClick = (filter: GetV2BuilderSearchFilterAnyOfItem) => { |
There was a problem hiding this comment.
Maybe useCallback and for other functions as well
There was a problem hiding this comment.
@0ubbe told me to skip useCallback unless there's a real performance issue. and these are just simple toggle functions with a few filter chips, so the overhead of useCallback probably isn't worth it here.
There was a problem hiding this comment.
Is this the proper way? Can't this just go to types.ts?
There was a problem hiding this comment.
Yaa... sorry - in hurry I named it constants instead of types - will change it
| > | ||
| {searchResults.map((item: SearchResponseItemsItem, index: number) => { | ||
| const { type, data } = getSearchItemType(item); | ||
| // backend give support to these 3 types only [right now] - we need to give support to integration and ai agent types in follow up PRs |
There was a problem hiding this comment.
What do you mean? Is there something missing? From backend perspective block == integration and I'm unsure what you mean by "ai agent" as in AI suggestion?
There was a problem hiding this comment.
In designs, search blocks come in two more types. I’ve attached an image of them.
In the integration block, I should receive the image URL and a unique name. In the AI block (sorry not the agent), I get the name of the AI when I search for LLM in the search bar.
I’m only receiving these types of responses from the backend.
/**
* Generated by orval v7.13.0 🍺
* Do not edit manually.
* AutoGPT Agent Server
* This server is used to execute agents that are created by the AutoGPT system.
* OpenAPI spec version: 0.1
*/
import type { BlockInfo } from "./blockInfo";
import type { LibraryAgent } from "./libraryAgent";
import type { StoreAgent } from "./storeAgent";
export type SearchResponseItemsItem = BlockInfo | LibraryAgent | StoreAgent;
| INITIAL_CREATORS_TO_SHOW, | ||
| ); | ||
|
|
||
| const handleLocalCategoryChange = ( |
There was a problem hiding this comment.
Again imo would be good to use useCallback for these
There was a problem hiding this comment.
I think the same above reason. No need.
github-project-automation
bot
moved this from 🆕 Needs initial review
to 🚧 Needs work
in AutoGPT development kanban
Changes SummaryThis PR adds filtering functionality to the blocks menu, allowing users to filter search results by category and creator. The changes include new filter components (BlockMenuFilters, FilterSheet), state management updates to blockMenuStore, and a refactoring of the custom-mutator to simplify query parameter handling by removing the separate params option and relying on pre-encoded URLs. Type: feature Components Affected: custom-mutator.ts, BlockMenu components, blockMenuStore, BlockMenuSearchContent Architecture Impact
Risk Areas: Query parameter encoding change - removed URLSearchParams handling in customMutator, now relies on pre-encoded URLs. This could break if any callers still pass params separately, The redirect handling (3xx responses) was added - need to verify this doesn't break existing redirect scenarios, State management complexity increased with new filter state in blockMenuStore Suggestions
Full review in progress... | Powered by diffray |
| const fullUrl = `${baseUrl}${url}`; | ||
|
|
||
| if (environment.isServerSide()) { | ||
| try { |
There was a problem hiding this comment.
🔴 CRITICAL - fetch() call missing error handling
Category: bug
Description:
The fetch() call is not wrapped in a try-catch block. Network errors, CORS errors, DNS failures, or other runtime exceptions will cause unhandled promise rejections.
Suggestion:
Wrap the fetch call in a try-catch block and throw an ApiError with appropriate context for network failures.
Confidence: 92%
Rule: bug_missing_try_catch
| }; | ||
|
|
||
| export const customMutator = async < | ||
| T extends { data: any; status: number; headers: Headers }, |
There was a problem hiding this comment.
🟠 HIGH - Generic type parameter uses 'any' which disables type checking
Category: quality
Description:
The type parameter T extends { data: any; status: number; headers: Headers } uses 'any' for the data field, which completely disables type checking for response data.
Suggestion:
Replace 'any' with a generic type parameter. Change the signature to use a separate generic type parameter TData instead of any to allow callers to specify the expected response data type while maintaining type safety.
Confidence: 85%
Rule: ts_prefer_specific_types_over_any_unknown_w
| const requestOptions = options; | ||
| const method = (requestOptions.method || "GET") as | ||
| | "GET" | ||
| | "POST" |
There was a problem hiding this comment.
🟠 HIGH - Type assertion on headers without runtime validation
Category: quality
Description:
The code uses (requestOptions.headers as Record<string, string>) without validating headers is actually a Record. RequestInit headers can be Headers, string[][], or Record<string, string>.
Suggestion:
Add proper type handling for all possible header types using instanceof Headers check and Array.isArray check before spreading.
Confidence: 72%
Rule: ts_type_assertion_abuse
| T extends { data: any; status: number; headers: Headers }, | ||
| >( | ||
| url: string, | ||
| options: RequestInit & { | ||
| params?: any; | ||
| } = {}, | ||
| options: RequestInit, |
There was a problem hiding this comment.
🟡 MEDIUM - Type assertions without runtime validation for response body
Category: quality
Description:
The getBody function uses type assertions as Promise<T> for blob and text responses without validating that the content matches type T.
Suggestion:
Return Promise<unknown> from getBody and use type guards at call sites to validate response types.
Confidence: 68%
Rule: ts_type_assertion_abuse
| const fullUrl = `${baseUrl}${url}`; | ||
|
|
||
| if (environment.isServerSide()) { | ||
| try { |
There was a problem hiding this comment.
🟡 MEDIUM - Console.warn for auth token failure
Category: quality
Description:
Console.warn logs server auth token retrieval failures to production console, potentially leaking security-related information.
Suggestion:
Replace with a proper logging service that supports environment-based log levels.
Confidence: 60%
Rule: fe_console_in_production
Review Summary
Validated 12 issues: 10 kept (bug, quality concerns), 2 filtered (low value console statements) Issues Found: 10See 5 individual line comment(s) for details. 📊 4 unique issue type(s) across 10 location(s) 📋 Full issue list (click to expand)🔴 CRITICAL - fetch() call missing error handling (2 occurrences)Category: bug 📍 View all locations
Rule: 🟠 HIGH - Generic type parameter uses 'any' which disables type checking (2 occurrences)Category: quality 📍 View all locations
Rule: 🟠 HIGH - Console.error in production before throwing ApiError (2 occurrences)Category: quality 📍 View all locations
Rule: 🟠 HIGH - Type assertion on headers without runtime validation (4 occurrences)Category: quality 📍 View all locations
Rule: Powered by diffray - Multi-Agent Code Review Agent |
Changes 🏗️
This PR adds filtering functionality to the new blocks menu, allowing users to filter search results by category and creator.
New Components:
BlockMenuFilters: Main filter component displaying active filters and filter chipsFilterSheet: Slide-out panel for selecting filters with categories and creatorsBlockMenuSearchContent: Refactored search results display componentFeatures Added:
State Management:
blockMenuStorewith filter state managementfilters,creators,creators_list, andcategoryCountsto storefilterandby_creatorparameters)Refactoring:
BlockMenuSearchtoBlockMenuSearchContentuseBlockMenuSearchtouseBlockMenuSearchContentBlockMenuSearchContentdirectoryAPI Changes:
custom-mutator.tsto properly handle query parameter encodingChecklist 📋
For code changes: