Skip to content

Conversation

@Wanyi9988
Copy link

Description

This PR fixes the issue where the search box in the tools overview was not working in list view.

Fixes #5469

Before

  • Typing in list view did not filter tools
image

After

  • Typing in the search box filters tools correctly regardless of view mode
image

Notes

  • The fix modifies the search filter logic to apply in list view
  • No changes to other functionality

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Wanyi9988, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug where the search functionality for the tools overview was non-operational in the list view. By integrating the existing filterTools logic directly into the data provided to the ToolsTable component, the change ensures that users can now effectively search and filter their tools regardless of their preferred display mode, significantly improving usability and consistency within the application.

Highlights

  • Bug Fix: Resolved an issue where the search box in the tools overview was not filtering results when the view was set to 'list view'.
  • Search Functionality: The search filter logic has been updated to correctly apply filtering to the tools list, ensuring consistent search behavior across all view modes.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes an issue where the search filter was not applied to the tools list view. The change correctly adds the filtering logic, making the list view consistent with the card view. My review points out a significant issue with the current search implementation: it's performed client-side on paginated data, which leads to an incorrect search experience as it only searches the current page. I've recommended moving the filtering logic to the server for correct behavior. Additionally, I've suggested using useMemo to optimize performance and reduce code duplication for the filtering logic.

</Box>
) : (
<ToolsTable data={getAllToolsApi.data.data} isLoading={isLoading} onSelect={edit} />
<ToolsTable data={getAllToolsApi.data.data.filter(filterTools)} isLoading={isLoading} onSelect={edit} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While this change correctly applies the filter to the list view, making it consistent with the card view, there are some improvements to consider for the overall filtering implementation:

  1. Client-side filtering on paginated data: The most significant issue is that filtering is performed on the client-side on paginated data. This means the search only applies to the tools on the currently displayed page. If a user searches for a tool that exists but is on a different page, it will not be found. This can be very confusing. For the search to work as expected, it should be implemented on the server-side. The search term should be passed to the API, which would then return the correctly filtered and paginated results.

  2. Performance and Code Duplication: The .filter() operation is executed on every render, and the same logic is duplicated for the card view on line 243. To improve performance and keep the code DRY, you can memoize the filtered data using useMemo and reuse it.

Here's an example of how you could use useMemo:

const filteredData = useMemo(() => {
  if (!getAllToolsApi.data?.data) return [];
  return getAllToolsApi.data.data.filter(filterTools);
}, [getAllToolsApi.data?.data, search]);

// Then use `filteredData` for both card and list views.

While the server-side filtering is a larger change, applying useMemo is a smaller refactor that would improve the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Search box in tools overview is not working

1 participant