Skip to content
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

feat(query): Add a "Search" tab in the Sidebar for wildcard queries. #107

Merged
merged 27 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5094abf
refactor SearchTabPanel from new log viewer folder back to current lo…
Henry8192 Oct 24, 2024
dcada93
Rewrite the ResultsGroup return type
Henry8192 Oct 25, 2024
ac22041
show queryResults in search tab panel
Henry8192 Oct 28, 2024
908b6db
fix search results not showing; show contexts before and after search…
Henry8192 Oct 31, 2024
b931505
Nasty solution for individual search results expansion control
Henry8192 Nov 1, 2024
3ddd96a
limit max query results to 1000; add jsdoc for ResultsGroup
Henry8192 Nov 4, 2024
2cbb032
change ResultsGroup structure: each should be a page of results; rewr…
Henry8192 Nov 4, 2024
0889361
amend stylelint
Henry8192 Nov 4, 2024
d8cd28c
edit QUERY_RESULT protocol to show search progress
Henry8192 Nov 5, 2024
4709312
display search progress in status bar
Henry8192 Nov 7, 2024
a8a1551
move search progress bar under search input box
Henry8192 Nov 7, 2024
67957ca
search results now support jump to current log event
Henry8192 Nov 7, 2024
640b3e3
address issues in code review
Henry8192 Nov 11, 2024
477ded6
Rename "search" -> "query" where appropriate; fix CSS issues.
Henry8192 Nov 11, 2024
e1dc2df
Fix panel overflow when there are a few query results; change progres…
Henry8192 Nov 11, 2024
ec8cc58
clean up sx props to css files
Henry8192 Nov 12, 2024
96606b7
Apply suggestions from code review
Henry8192 Nov 12, 2024
854293f
Resolve issues from code review
Henry8192 Nov 12, 2024
30481f9
fix lint
Henry8192 Nov 12, 2024
8a81a87
Apply suggestions from code review
Henry8192 Nov 14, 2024
2c10ab7
resolve rest of suggestions
Henry8192 Nov 14, 2024
fb12080
fix lint
Henry8192 Nov 14, 2024
34a8043
Merge branch 'main' into search-tab-panel
Henry8192 Nov 14, 2024
f71269e
Update src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabP…
Henry8192 Nov 15, 2024
e002f4f
Merge branch 'main' into search-tab-panel
junhaoliao Nov 15, 2024
8793d0b
Rename `DecodeResultType` -> `DecodeResult` as a result of merging fr…
junhaoliao Nov 15, 2024
580b876
Merge branch 'main' into search-tab-panel
Henry8192 Nov 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";

import {
ButtonGroup,
DialogContent,
DialogTitle,
TabPanel,
Expand All @@ -14,6 +15,7 @@ interface CustomTabPanelProps {
children: React.ReactNode,
tabName: string,
title: string,
titleButtons?: React.ReactNode,
}

/**
Expand All @@ -23,9 +25,15 @@ interface CustomTabPanelProps {
* @param props.children
* @param props.tabName
* @param props.title
* @param props.titleButtons
* @return
*/
const CustomTabPanel = ({children, tabName, title}: CustomTabPanelProps) => {
const CustomTabPanel = ({
children,
tabName,
title,
titleButtons,
}: CustomTabPanelProps) => {
return (
<TabPanel
className={"sidebar-tab-panel"}
Expand All @@ -38,6 +46,13 @@ const CustomTabPanel = ({children, tabName, title}: CustomTabPanelProps) => {
>
{title}
</Typography>
<ButtonGroup
size={"sm"}
spacing={"1px"}
variant={"plain"}
>
{titleButtons}
</ButtonGroup>
</DialogTitle>
<DialogContent>
{children}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
.result-button {
user-select: none;

overflow: hidden;
display: flex !important;

text-overflow: ellipsis;
white-space: nowrap;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider removing the !important flag from the display property.

The use of !important suggests potential specificity issues. Consider increasing selector specificity or restructuring CSS to avoid specificity wars.

-    display: flex !important;
+    display: flex;

If you need to override conflicting styles, consider using a more specific selector like:

.sidebar .search-tab-panel .result-button {
    display: flex;
}


.result-text {
display: inline !important;
font-size: 0.875rem !important;
font-weight: 400 !important;
}

.before-match,
.match,
.after-match {
flex: 1 1 0; /* Allow each item to take an equal part */

Check failure on line 20 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.css

View workflow job for this annotation

GitHub Actions / lint-check

Stylelint problem

Delete "·" (prettier/prettier)
overflow: hidden;

Check failure on line 21 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.css

View workflow job for this annotation

GitHub Actions / lint-check

Stylelint problem

Expected "overflow" to come before "flex" (order/properties-order)
text-overflow: ellipsis;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider performance optimizations for text truncation.

Given the PR objectives mentioning browser freezing with large search results, consider:

  1. Using CSS containment to optimize rendering
  2. Adding will-change for smoother animations if transitions are planned
 .before-match,
 .match,
 .after-match {
-    flex: 1 1 0;  /* Allow each item to take an equal part */
-    overflow: hidden;
+    flex: 1 1 0;
+    contain: content;
+    overflow: hidden;
     text-overflow: ellipsis;
 }

Also, fix the linting issues by reordering properties:

 .before-match,
 .match,
 .after-match {
+    overflow: hidden;
     flex: 1 1 0;
-    overflow: hidden;
     text-overflow: ellipsis;
 }

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 GitHub Check: lint-check

[failure] 20-20: Stylelint problem
Delete "·" (prettier/prettier)


[failure] 21-21: Stylelint problem
Expected "overflow" to come before "flex" (order/properties-order)

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import {
ListItemButton,
Typography,
} from "@mui/joy";

import "./Result.css";


interface ResultProps {
message: string,
matchRange: [number, number]
}

/**
* Displays a button containing a message, which highlights a specific range of text.
*
* @param props
* @param props.message
* @param props.matchRange A two-element array indicating the start and end indices of the substring
* to be highlighted.
* @return
*/
const Result = ({message, matchRange}: ResultProps) => {
const [
beforeMatch,
match,
afterMatch,
] = [
message.slice(0, matchRange[0]),
message.slice(...matchRange),
message.slice(matchRange[1]),
];

return (
<ListItemButton className={"result-button"}>
<Typography
className={"result-text before-match"}
level={"body-xs"}
>
{beforeMatch}
</Typography>
<Typography
className={"result-text match"}
level={"body-xs"}
sx={{
backgroundColor: "warning.softBg",
}}
>
{match}
</Typography>
<Typography
className={"result-text after-match"}
level={"body-xs"}
>
{afterMatch}
</Typography>
</ListItemButton>
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance accessibility and error handling.

The component needs improvements in:

  1. Accessibility attributes for screen readers
  2. Keyboard navigation support
  3. Error boundaries for invalid ranges
-    <ListItemButton className={"result-button"}>
+    <ListItemButton
+        className={"result-button"}
+        role="option"
+        aria-selected={false}
+        onKeyDown={(e) => {
+            if (e.key === 'Enter' || e.key === ' ') {
+                // Handle selection
+            }
+        }}
+    >
         <Typography
             className={"result-text before-match"}
             level={"body-xs"}
+            component="span"
         >
             {beforeMatch}
         </Typography>
         <Typography
             className={"result-text match"}
             level={"body-xs"}
+            component="mark"
             sx={{
                 backgroundColor: "warning.softBg",
             }}
+            aria-label={`Matched text: ${match}`}
         >
             {match}
         </Typography>

};

export default Result;
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
.results-group-title-button {
flex-direction: row-reverse !important;
gap: 2px !important;
padding-inline-start: 0 !important;
}

.results-group-title-container {
display: flex;
flex-grow: 1;
}

.results-group-title-text-container {
flex-grow: 1;
gap: 0.1rem;
align-items: center;
}

.results-group-content {
margin-left: 1.5px !important;
border-left: 1px solid var(--joy-palette-neutral-outlinedBorder, #cdd7e1);

Check failure on line 20 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.css

View workflow job for this annotation

GitHub Actions / lint-check

Stylelint problem

Expected custom property name "--joy-palette-neutral-outlinedBorder" to be kebab-case - https://stylelint.io/user-guide/rules/custom-property-pattern
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import {
useEffect,
useState,
} from "react";
import * as React from "react";

import {
Accordion,
AccordionDetails,
AccordionSummary,
Box,
Chip,
List,
Stack,
Typography,
} from "@mui/joy";

import DescriptionOutlinedIcon from "@mui/icons-material/DescriptionOutlined";

import {QueryResults} from "../../../../../typings/worker";
import Result from "./Result";

import "./ResultsGroup.css";


interface ResultsGroupProps {
isAllExpanded: boolean,
queryResults: QueryResults,
}

/**

Check warning on line 31 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx

View workflow job for this annotation

GitHub Actions / lint-check

Missing JSDoc block description

Check warning on line 31 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx

View workflow job for this annotation

GitHub Actions / lint-check

Missing JSDoc @return declaration
*
* @param props
* @param props.isAllExpanded
* @param props.queryResults
*/
const ResultsGroup = ({
isAllExpanded,
queryResults,
}: ResultsGroupProps) => {
const [expandedMap, setExpandedMap] = useState<Record<number, boolean>>({});
const handleAccordionChange = (
pageNum: number
) => (_: React.SyntheticEvent, newValue: boolean) => {
setExpandedMap((prev) => ({
...prev,
[pageNum]: newValue,
}));
};

// On `isAllExpanded` updates, sync current results group's expand status.
useEffect(() => {
const newExpandedMap = Object.fromEntries(
Object.entries(expandedMap).map(([key]) => [key,
isAllExpanded])
);

setExpandedMap(newExpandedMap);
}, [isAllExpanded]);

Check warning on line 59 in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx

View workflow job for this annotation

GitHub Actions / lint-check

React Hook useEffect has a missing dependency: 'expandedMap'. Either include it or remove the dependency array
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Include expandedMap in useEffect dependency array or refactor effect

The useEffect hook uses expandedMap but doesn't include it in the dependency array. This might lead to unexpected behaviour due to stale closures. Consider including expandedMap in the dependencies or refactor the effect.

Option 1: Include expandedMap in the dependency array:

-        }, [isAllExpanded]);
+        }, [isAllExpanded, expandedMap]);

Option 2: Refactor the useEffect to use a functional state update, avoiding the need for expandedMap in dependencies:

        useEffect(() => {
-            const newExpandedMap = Object.fromEntries(
-                Object.entries(expandedMap).map(([key]) => [key, isAllExpanded])
-            );
-
-            setExpandedMap(newExpandedMap);
+            setExpandedMap((prevExpandedMap) => {
+                const newExpandedMap = Object.fromEntries(
+                    Object.keys(prevExpandedMap).map((key) => [key, isAllExpanded])
+                );
+                return newExpandedMap;
+            });
        }, [isAllExpanded]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
const newExpandedMap = Object.fromEntries(
Object.entries(expandedMap).map(([key]) => [key,
isAllExpanded])
);
setExpandedMap(newExpandedMap);
}, [isAllExpanded]);
useEffect(() => {
setExpandedMap((prevExpandedMap) => {
const newExpandedMap = Object.fromEntries(
Object.keys(prevExpandedMap).map((key) => [key, isAllExpanded])
);
return newExpandedMap;
});
}, [isAllExpanded]);
🧰 Tools
🪛 GitHub Check: lint-check

[warning] 59-59:
React Hook useEffect has a missing dependency: 'expandedMap'. Either include it or remove the dependency array


useEffect(() => {
setExpandedMap((prevMap) => {
const updatedMap = {...prevMap};
queryResults.forEach((_, pageNum) => {
// Only set for entries not already in expandedMap
if (!(pageNum in updatedMap)) {
updatedMap[pageNum] = isAllExpanded;
}
});

return updatedMap;
});
}, [
isAllExpanded,
queryResults,
]);

return (
<>
{Array.from(queryResults.entries()).map(([pageNum, results]) => (
<Accordion
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement immediate performance optimizations

Given the reported browser freezing issues, here are some immediate optimizations to implement while working on virtualization:

  1. Batch render updates
  2. Add result limit with "Show More" button
+const INITIAL_RESULTS_LIMIT = 50;
+
 const ResultsGroup = ({
     isAllExpanded,
     queryResults,
 }: ResultsGroupProps) => {
+    const [resultsLimit, setResultsLimit] = useState(INITIAL_RESULTS_LIMIT);
+    
     // ... existing code ...
     
     return (
         <>
             {Array.from(queryResults.entries()).map(([pageNum, results]) => (
                 <Accordion
                     // ... existing props ...
                 >
                     // ... existing AccordionSummary ...
                     <AccordionDetails className={"results-group-content"}>
                         <List size={"sm"}>
-                            {results.map((r) => (
+                            {results.slice(0, resultsLimit).map((r) => (
                                 <Result
                                     key={r.logEventNum}
                                     matchRange={r.matchRange}
                                     message={r.message}/>
                             ))}
+                            {results.length > resultsLimit && (
+                                <Button
+                                    onClick={() => setResultsLimit(prev => prev + INITIAL_RESULTS_LIMIT)}
+                                    variant="outlined"
+                                    fullWidth
+                                >
+                                    Show More Results
+                                </Button>
+                            )}
                         </List>
                     </AccordionDetails>
                 </Accordion>
             ))}
         </>
     );
 };

Also applies to: 114-119

expanded={expandedMap[pageNum] ?? isAllExpanded}
key={pageNum}
onChange={handleAccordionChange(pageNum)}
>
<AccordionSummary
slotProps={{
button: {className: "results-group-title-button"},
}}
>
<Box className={"results-group-title-container"}>
<Stack
className={"results-group-title-text-container"}
direction={"row"}
>
<DescriptionOutlinedIcon fontSize={"inherit"}/>
<Typography
level={"title-sm"}
>
Page
{" "}
{pageNum}
</Typography>
</Stack>
<Chip size={"sm"}>
{results.length}
</Chip>
</Box>
</AccordionSummary>
<AccordionDetails className={"results-group-content"}>
<List size={"sm"}>
{results.map((r) => (
<Result
key={r.logEventNum}
matchRange={r.matchRange}
message={r.message}/>
))}
</List>
</AccordionDetails>
</Accordion>
))}
</>
);
};

export default ResultsGroup;
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import {
useContext,
useRef,
useState,
} from "react";

import {
AccordionGroup,
IconButton,
Textarea,
ToggleButtonGroup,
} from "@mui/joy";

import UnfoldLessIcon from "@mui/icons-material/UnfoldLess";
import UnfoldMoreIcon from "@mui/icons-material/UnfoldMore";

import {StateContext} from "../../../../../contexts/StateContextProvider";
import {
TAB_DISPLAY_NAMES,
TAB_NAME,
} from "../../../../../typings/tab";
import CustomTabPanel from "../CustomTabPanel";
import TitleButton from "../TitleButton";
import ResultsGroup from "./ResultsGroup";


enum SEARCH_OPTION {
IS_CASE_SENSITIVE = "isCaseSensitive",
IS_REGEX = "isRegex"
}

/**
* Displays a panel for submitting search queries and viewing query results.
*
* @return
*/
const SearchTabPanel = () => {
const [isAllExpanded, setIsAllExpanded] = useState<boolean>(true);
const [searchOptions, setSearchOptions] = useState<SEARCH_OPTION[]>([]);
const searchTextRef = useRef<HTMLTextAreaElement>(null);
const {queryResults, startQuery} = useContext(StateContext);
const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
if ("Enter" === event.key && searchTextRef.current) {
event.preventDefault();
const isCaseSensitive = searchOptions.includes(SEARCH_OPTION.IS_CASE_SENSITIVE);
const isRegex = searchOptions.includes(SEARCH_OPTION.IS_REGEX);
startQuery(searchTextRef.current.value, isRegex, isCaseSensitive);
}
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add performance optimizations and error handling.

Given the reported issues with browser freezing on large result sets, consider implementing:

  1. Debounce the search operation to prevent excessive queries
  2. Add loading state to provide user feedback
  3. Implement error handling for failed searches

Here's a suggested implementation:

+ import { useState, useRef, useContext, useCallback } from 'react';
+ import debounce from 'lodash/debounce';

  const SearchTabPanel = () => {
    const [isAllExpanded, setIsAllExpanded] = useState<boolean>(true);
    const [searchOptions, setSearchOptions] = useState<SEARCH_OPTION[]>([]);
+   const [isLoading, setIsLoading] = useState<boolean>(false);
+   const [error, setError] = useState<string | null>(null);
    const searchTextRef = useRef<HTMLTextAreaElement>(null);
    const {queryResults, startQuery} = useContext(StateContext);

-   const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
+   const executeSearch = useCallback(async (searchText: string) => {
+     try {
+       setIsLoading(true);
+       setError(null);
        const isCaseSensitive = searchOptions.includes(SEARCH_OPTION.IS_CASE_SENSITIVE);
        const isRegex = searchOptions.includes(SEARCH_OPTION.IS_REGEX);
-       startQuery(searchTextRef.current.value, isRegex, isCaseSensitive);
+       await startQuery(searchText, isRegex, isCaseSensitive);
+     } catch (err) {
+       setError(err instanceof Error ? err.message : 'Search failed');
+     } finally {
+       setIsLoading(false);
+     }
+   }, [searchOptions, startQuery]);

+   const debouncedSearch = useCallback(
+     debounce((searchText: string) => executeSearch(searchText), 300),
+     [executeSearch]
+   );

+   const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
      if ("Enter" === event.key && searchTextRef.current) {
        event.preventDefault();
+       debouncedSearch(searchTextRef.current.value);
      }
    };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [isAllExpanded, setIsAllExpanded] = useState<boolean>(true);
const [searchOptions, setSearchOptions] = useState<SEARCH_OPTION[]>([]);
const searchTextRef = useRef<HTMLTextAreaElement>(null);
const {queryResults, startQuery} = useContext(StateContext);
const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
if ("Enter" === event.key && searchTextRef.current) {
event.preventDefault();
const isCaseSensitive = searchOptions.includes(SEARCH_OPTION.IS_CASE_SENSITIVE);
const isRegex = searchOptions.includes(SEARCH_OPTION.IS_REGEX);
startQuery(searchTextRef.current.value, isRegex, isCaseSensitive);
}
};
const [isAllExpanded, setIsAllExpanded] = useState<boolean>(true);
const [searchOptions, setSearchOptions] = useState<SEARCH_OPTION[]>([]);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [error, setError] = useState<string | null>(null);
const searchTextRef = useRef<HTMLTextAreaElement>(null);
const {queryResults, startQuery} = useContext(StateContext);
const executeSearch = useCallback(async (searchText: string) => {
try {
setIsLoading(true);
setError(null);
const isCaseSensitive = searchOptions.includes(SEARCH_OPTION.IS_CASE_SENSITIVE);
const isRegex = searchOptions.includes(SEARCH_OPTION.IS_REGEX);
await startQuery(searchText, isRegex, isCaseSensitive);
} catch (err) {
setError(err instanceof Error ? err.message : 'Search failed');
} finally {
setIsLoading(false);
}
}, [searchOptions, startQuery]);
const debouncedSearch = useCallback(
debounce((searchText: string) => executeSearch(searchText), 300),
[executeSearch]
);
const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
if ("Enter" === event.key && searchTextRef.current) {
event.preventDefault();
debouncedSearch(searchTextRef.current.value);
}
};

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance search handler with debouncing and error handling.

The current implementation may cause performance issues with large result sets. Consider implementing:

  1. Debounce the search operation
  2. Add loading state
  3. Implement error handling
  4. Validate regex patterns
+import { debounce } from 'lodash';
+
 const SearchTabPanel = () => {
+    const [isLoading, setIsLoading] = useState(false);
+    const [error, setError] = useState<string | null>(null);
+
+    const executeSearch = useCallback(async (
+        searchText: string,
+        isRegex: boolean,
+        isCaseSensitive: boolean
+    ) => {
+        try {
+            if (isRegex) {
+                new RegExp(searchText); // Validate regex
+            }
+            setIsLoading(true);
+            setError(null);
+            await startQuery(searchText, isRegex, isCaseSensitive);
+        } catch (err) {
+            setError(err instanceof Error ? err.message : 'Search failed');
+        } finally {
+            setIsLoading(false);
+        }
+    }, [startQuery]);
+
+    const debouncedSearch = useMemo(
+        () => debounce(executeSearch, 300),
+        [executeSearch]
+    );
+
     const handleSearch = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
         if ("Enter" === event.key && searchTextRef.current) {
             event.preventDefault();
             const isCaseSensitive = searchOptions.includes(SEARCH_OPTION.IS_CASE_SENSITIVE);
             const isRegex = searchOptions.includes(SEARCH_OPTION.IS_REGEX);
-            startQuery(searchTextRef.current.value, isRegex, isCaseSensitive);
+            debouncedSearch(searchTextRef.current.value, isRegex, isCaseSensitive);
         }
     };

Committable suggestion skipped: line range outside the PR's diff.


return (
<CustomTabPanel
tabName={TAB_NAME.SEARCH}
title={TAB_DISPLAY_NAMES[TAB_NAME.SEARCH]}
titleButtons={
<TitleButton onClick={() => { setIsAllExpanded((v) => !v); }}>
{isAllExpanded ?
<UnfoldLessIcon/> :
<UnfoldMoreIcon/>}
</TitleButton>
}
>
<Textarea
maxRows={7}
placeholder={"Search"}
size={"sm"}
sx={{flexDirection: "row"}}
endDecorator={
<>
<ToggleButtonGroup
size={"sm"}
spacing={0.3}
sx={{borderRadius: "2px"}}
value={searchOptions}
variant={"plain"}
onChange={(_, newValue) => {
setSearchOptions(newValue);
}}
>
<IconButton
sx={{fontFamily: "Inter"}}
value={SEARCH_OPTION.IS_CASE_SENSITIVE}
>
Aa
</IconButton>
<IconButton
sx={{fontFamily: "Inter"}}
value={SEARCH_OPTION.IS_REGEX}
>
.*
</IconButton>
</ToggleButtonGroup>
</>
}
slotProps={{
textarea: {ref: searchTextRef},
endDecorator: {sx: {marginBlockStart: 0, display: "block"}},
}}
onKeyDown={handleSearch}/>
<AccordionGroup
disableDivider={true}
size={"sm"}
>
<ResultsGroup
isAllExpanded={isAllExpanded}
queryResults={queryResults}/>
</AccordionGroup>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize rendering performance for large result sets.

To address the re-rendering issues mentioned in the PR comments, consider implementing:

  1. Memoization of the ResultsGroup component
  2. Virtualization for large result sets
+ import { memo } from 'react';
+ import { FixedSizeList } from 'react-window';

- <AccordionGroup
-     disableDivider={true}
-     size={"sm"}
- >
-     <ResultsGroup
-         isAllExpanded={isAllExpanded}
-         queryResults={queryResults}/>
- </AccordionGroup>

+ const MemoizedResultsGroup = memo(ResultsGroup);
+ 
+ <AccordionGroup
+     disableDivider={true}
+     size={"sm"}
+ >
+     <MemoizedResultsGroup
+         isAllExpanded={isAllExpanded}
+         queryResults={queryResults}/>
+ </AccordionGroup>

Consider implementing virtualization if the results can be large:

interface ResultRowProps {
  index: number;
  style: React.CSSProperties;
  data: {
    results: typeof queryResults;
    isExpanded: boolean;
  };
}

const ResultRow = memo(({ index, style, data }: ResultRowProps) => {
  const result = data.results[index];
  return (
    <div style={style}>
      <MemoizedResultsGroup
        isAllExpanded={data.isExpanded}
        queryResults={[result]}
      />
    </div>
  );
});

// In the render method:
<FixedSizeList
  height={400}
  width="100%"
  itemCount={queryResults.length}
  itemSize={50}
  itemData={{ results: queryResults, isExpanded: isAllExpanded }}
>
  {ResultRow}
</FixedSizeList>

</CustomTabPanel>
);
};

export default SearchTabPanel;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.sidebar-tab-title-button {
min-width: 0 !important;
min-height: 0 !important;
}
Loading
Loading