fix: memory leak when toggling search panel

[RohitKushvaha01]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @RohitKushvaha01, I'm Gemini Code Assist¹! 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 memory leak that occurred when repeatedly toggling the search panel. The core fix involves ensuring that the history navigation setup is executed only once, preventing redundant initialization and associated resource consumption. Additionally, the method for toggling the search panel via the Escape key has been updated to leverage a more centralized action stack, improving consistency.

Highlights

• Prevent duplicate history navigation setup: Introduced a historyNavigationInitialized flag to ensure the setupHistoryNavigation function runs only once. This prevents potential memory leaks or redundant event listener registrations that could occur when the search panel is toggled multiple times.
• Refactor search panel toggle logic: Updated the Escape key press handler to use actionStack.get("search-bar")?.action() instead of a direct toggleSearch() call. This promotes a more consistent and potentially more robust way to manage the search panel's state through a centralized action mechanism.

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. ↩

[gemini-code-assist]

Code Review

This pull request addresses a memory leak that occurred when toggling the search panel. The fix prevents event listeners from being added multiple times by using a flag, which is the correct approach to solve the issue. I've also noticed a change in behavior for the Ctrl+F shortcut within the search input. It now always closes the search panel, whereas it previously could update the search term from the editor's selection. I've left a comment to discuss whether this change was intentional, as the previous functionality could be useful.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR successfully fixes a memory leak that occurred when repeatedly toggling the search panel. The issue was caused by setupHistoryNavigation() being called every time the search panel opened, attaching duplicate event listeners to the same cached DOM elements.

Changes Made

1. Added initialization guard: Introduced a historyNavigationInitialized flag that ensures event listeners are only attached once to the search input elements, preventing the memory leak.

2. Updated Ctrl+F behavior: Changed from calling toggleSearch() to directly calling actionStack.get("search-bar")?.action() when Ctrl+F is pressed inside the search input, ensuring the search panel always closes (maintaining the behavior from PR feat: close search dialog with ctrl+f #1807).

How the Fix Works

The search input elements ($searchRow1 and $searchRow2) are cached by the quickTools component and reused across multiple open/close cycles. Previously, each time the search panel was opened, new event listeners were attached to these same elements, causing a memory leak. The flag-based guard prevents this by ensuring the listeners are only attached on the first initialization.

When the search panel is closed via removeSearch(), the DOM elements are removed from the document but not destroyed. The event listeners remain attached to these detached elements, which is the correct behavior since the elements are reattached when the search panel reopens.

Confidence Score: 5/5

• This PR is safe to merge with no identified issues
• The fix correctly addresses the memory leak by preventing duplicate event listener attachments. The implementation uses a standard flag-based initialization pattern that is appropriate for this use case. Both changes are minimal, focused, and well-tested through code analysis. The search input elements are cached and reused, making the single-initialization approach correct. No edge cases or potential bugs were identified during thorough exploration of the codebase.
• No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
src/handlers/quickTools.js	5/5	Correctly fixes memory leak by preventing duplicate event listener attachments; uses flag-based initialization guard and updates Ctrl+F behavior to always close search bar

Sequence Diagram

sequenceDiagram
    participant User
    participant toggleSearch
    participant setupHistoryNavigation
    participant searchInput
    participant actionStack
    participant removeSearch

    Note over User,removeSearch: First Toggle (Open Search)
    User->>toggleSearch: Open search panel
    toggleSearch->>searchInput: Create/retrieve cached search rows
    toggleSearch->>setupHistoryNavigation: Call setupHistoryNavigation()
    setupHistoryNavigation->>setupHistoryNavigation: Check historyNavigationInitialized flag
    Note over setupHistoryNavigation: flag is false, proceed
    setupHistoryNavigation->>searchInput: addEventListener('keydown')
    setupHistoryNavigation->>searchInput: addEventListener('input')
    setupHistoryNavigation->>setupHistoryNavigation: Set historyNavigationInitialized = true
    toggleSearch->>actionStack: Push 'search-bar' action
    Note over searchInput,actionStack: Search panel is open with listeners attached

    Note over User,removeSearch: User presses Ctrl+F in search input
    User->>searchInput: Press Ctrl+F
    searchInput->>actionStack: Call actionStack.get('search-bar')?.action()
    actionStack->>removeSearch: Execute removal action
    removeSearch->>searchInput: Remove search rows from DOM
    Note over searchInput: DOM elements detached but not destroyed,<br/>listeners remain attached

    Note over User,removeSearch: Second Toggle (Reopen Search)
    User->>toggleSearch: Reopen search panel
    toggleSearch->>searchInput: Retrieve same cached search rows
    toggleSearch->>setupHistoryNavigation: Call setupHistoryNavigation()
    setupHistoryNavigation->>setupHistoryNavigation: Check historyNavigationInitialized flag
    Note over setupHistoryNavigation: flag is true, return early
    setupHistoryNavigation-->>toggleSearch: Skip listener attachment
    toggleSearch->>actionStack: Push 'search-bar' action
    Note over searchInput,actionStack: Search panel reopened,<br/>existing listeners still work,<br/>no memory leak!

Loading