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 all 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
Expand Up @@ -2,12 +2,19 @@
padding: 0.75rem;
}

.sidebar-tab-panel-container {
display: flex;
flex-direction: column;
height: 100%;
}

.sidebar-tab-panel-title-container {
user-select: none;
margin-bottom: 0.5rem !important;
}

.sidebar-tab-panel-title {
flex-grow: 1;
font-size: 0.875rem !important;
font-weight: 400 !important;
text-transform: uppercase;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from "react";

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

/**
Expand All @@ -23,25 +26,40 @@ 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"}
value={tabName}
>
<DialogTitle className={"sidebar-tab-panel-title-container"}>
<Typography
className={"sidebar-tab-panel-title"}
level={"body-md"}
>
{title}
</Typography>
</DialogTitle>
<DialogContent>
{children}
</DialogContent>
<Box className={"sidebar-tab-panel-container"}>
<DialogTitle className={"sidebar-tab-panel-title-container"}>
<Typography
className={"sidebar-tab-panel-title"}
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
level={"body-md"}
>
{title}
</Typography>
<ButtonGroup
size={"sm"}
spacing={"1px"}
variant={"plain"}
>
{titleButtons}
</ButtonGroup>
</DialogTitle>
<DialogContent>
{children}
</DialogContent>
</Box>
</TabPanel>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.tab-panel-title-button {
min-width: 0 !important;
min-height: 0 !important;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {
IconButton,
IconButtonProps,
} from "@mui/joy";

import "./PanelTitleButton.css";


/**
* Renders an IconButton for use in sidebar tab titles.
*
* @param props
* @return
*/
const PanelTitleButton = (props: IconButtonProps) => {
const {className, ...rest} = props;
return (
<IconButton
className={`tab-panel-title-button ${className ?? ""}`}
{...rest}/>
);
};


Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
export default PanelTitleButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.result-button {
user-select: none;

overflow-x: hidden;

width: 100%;
padding-left: 12px;

text-align: left;
text-overflow: ellipsis;
white-space: nowrap;
}
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved

.result-button:hover {
cursor: default;
}

.result-button-text {
font-family: Inter, sans-serif !important;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import {
ListItemButton,
Typography,
} from "@mui/joy";

import {updateWindowUrlHashParams} from "../../../../../contexts/UrlContextProvider";

import "./Result.css";


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

const QUERY_RESULT_PREFIX_MAX_CHARACTERS = 20;

/**
* Renders a query result as a button with a message, highlighting the first matching text range.
*
* @param props
* @param props.message
* @param props.matchRange A two-element array [begin, end) representing the indices of the matching
* text range.
* @param props.logEventNum
* @return
*/
const Result = ({logEventNum, message, matchRange}: ResultProps) => {
const [
beforeMatch,
match,
afterMatch,
] = [
message.slice(0, matchRange[0]),
message.slice(...matchRange),
message.slice(matchRange[1]),
];
const handleResultButtonClick = () => {
updateWindowUrlHashParams({logEventNum});
};
Comment on lines +39 to +41
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to the click handler.

The click handler should handle potential errors when updating URL parameters.

     const handleResultButtonClick = () => {
+        try {
             updateWindowUrlHashParams({logEventNum});
+        } catch (error) {
+            console.error('Failed to update URL parameters:', error);
+            // Consider showing a user-friendly error message
+        }
     };

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


return (
<ListItemButton
className={"result-button"}
onClick={handleResultButtonClick}
>
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
<Typography
className={"result-button-text"}
level={"body-sm"}
>
<span>
{(QUERY_RESULT_PREFIX_MAX_CHARACTERS < beforeMatch.length) && "..."}
{beforeMatch.slice(-QUERY_RESULT_PREFIX_MAX_CHARACTERS)}
</span>
<Typography
className={"result-button-text"}
level={"body-sm"}
sx={{backgroundColor: "warning.softBg"}}
>
{match}
</Typography>
{afterMatch}
</Typography>
</ListItemButton>
);
};

Henry8192 marked this conversation as resolved.
Show resolved Hide resolved

export default Result;
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
.results-group-summary-button {
cursor: default !important;
flex-direction: row-reverse !important;
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
gap: 2px !important;
padding-inline-start: 0 !important;
}

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

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

.results-group-summary-count {
border-radius: 4px !important;
}

.results-group-details {
margin-left: 1.5px !important;
/* stylelint-disable-next-line custom-property-pattern */
border-left: 1px solid var(--joy-palette-neutral-outlinedBorder, #cdd7e1);
}

.results-group-details-content {
padding-block: 0 !important;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import React, {
memo,
useEffect,
useState,
} from "react";

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

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

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

import "./ResultsGroup.css";


interface ResultsGroupProps {
isAllExpanded: boolean,
pageNum: number,
results: QueryResultsType[],
}

/**
* Renders a group of results, where each group represents a list of results from a single page.
*
* @param props
* @param props.isAllExpanded
* @param props.pageNum
* @param props.results
* @return
*/
const ResultsGroup = memo(({
isAllExpanded,
pageNum,
results,
}: ResultsGroupProps) => {
const [isExpanded, setIsExpanded] = useState<boolean>(isAllExpanded);

const handleAccordionChange = (
_: React.SyntheticEvent,
newValue: boolean
) => {
setIsExpanded(newValue);
};

// On `isAllExpanded` update, sync current results group's expand status.
useEffect(() => {
setIsExpanded(isAllExpanded);
}, [isAllExpanded]);

return (
<Accordion
expanded={isExpanded}
onChange={handleAccordionChange}
>
<AccordionSummary
slotProps={{
button: {className: "results-group-summary-button"},
}}
>
<Box className={"results-group-summary-container"}>
<Stack
className={"results-group-summary-text-container"}
direction={"row"}
>
<DescriptionOutlinedIcon fontSize={"inherit"}/>
<Typography
fontFamily={"Inter"}
level={"title-sm"}
>
{"Page "}
{pageNum}
</Typography>
</Stack>
<Chip
className={"results-group-summary-count"}
size={"sm"}
variant={"solid"}
>
{results.length}
</Chip>
</Box>
</AccordionSummary>
<AccordionDetails
className={"results-group-details"}
slotProps={{content: {className: "results-group-details-content"}}}
>
<List size={"sm"}>
{results.map((r, index) => (
<Result
key={index}
logEventNum={r.logEventNum}
matchRange={r.matchRange}
message={r.message}/>
))}
Henry8192 marked this conversation as resolved.
Show resolved Hide resolved
</List>
Comment on lines +97 to +105
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Implement virtualization for large result sets

While the current implementation works, it may cause performance issues with large result sets. Consider implementing virtualization to improve performance.

Here's a suggested implementation using react-window:

import { FixedSizeList } from 'react-window';

const ResultsList = ({ results }) => (
  <List size="sm">
    <FixedSizeList
      height={400}
      width="100%"
      itemCount={results.length}
      itemSize={35}
    >
      {({ index, style }) => (
        <Result
          style={style}
          key={index}
          logEventNum={results[index].logEventNum}
          matchRange={results[index].matchRange}
          message={results[index].message}
        />
      )}
    </FixedSizeList>
  </List>
);

Would you like me to create a GitHub issue to track this performance enhancement?

</AccordionDetails>
</Accordion>
);
});

ResultsGroup.displayName = "ResultsGroup";

Henry8192 marked this conversation as resolved.
Show resolved Hide resolved

export default ResultsGroup;
Loading
Loading