-
Notifications
You must be signed in to change notification settings - Fork 82
feat(new-webui): Add search results table. #866
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
Conversation
WalkthroughThis update introduces a search results table to the log viewer web UI client. New components and supporting files are added for rendering search results with syntax-highlighted messages and links to an external log viewer. The table is currently populated with dummy data and uses Ant Design components for layout and styling. Supporting CSS modules and utility files are included for custom styling. The main search page is updated to visually group the search controls and the new results table. Dependency updates add syntax highlighting libraries and their type definitions to the client application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchPage
participant SearchControls
participant SearchResultsTable
participant Message
participant LogViewerLink
User->>SearchPage: Loads Search Page
SearchPage->>SearchControls: Render search controls
SearchPage->>SearchResultsTable: Render table with dummy results
SearchResultsTable->>Message: Render each message (with syntax highlighting)
Message->>LogViewerLink: Render link to external log viewer
User->>LogViewerLink: Clicks link
LogViewerLink->>External Log Viewer: Opens file in new tab
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
components/log-viewer-webui/client/src/pages/SearchPage/index.tsx (1)
13-16
: Component structure looks goodConsider using dot notation instead of bracket notation for accessing the CSS module class for better readability.
- <div className={styles["searchPageContainer"]}> + <div className={styles.searchPageContainer}>components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
16-38
: File path column is missing.The columns configuration only includes "Timestamp" and "Message" columns, but the
SearchResult
interface and dummy data both have afilePath
field. While the file path is included within the Message component, consider whether users would expect to see and sort by file path as a separate column.const searchResultsTableColumns: NonNullable<TableProps<SearchResult>["columns"]> = [ { dataIndex: "timestamp", key: "timestamp", sorter: true, title: "Timestamp", width: 15, }, { dataIndex: "message", key: "message", render: (_, record) => ( <Message filePath={record.filePath} message={record.message}/> ), title: "Message", width: 85, }, + { + dataIndex: "filePath", + key: "filePath", + title: "File Path", + width: 20, + sorter: true, + }, ];If adding this column, you'll need to adjust the width percentages to ensure they total 100.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (2)
12-15
: Consider making log viewer URL configurable.The hardcoded URL will need to be updated once the log viewer setup is complete. Consider making this a configurable environment variable or importing from a central configuration.
-// eslint-disable-next-line no-warning-comments -// TODO: Fix link to connect to package log viewer when log viewer setup finished. Also pass -// proper args to package log viewer. -const LOG_VIEWER_URL = "https://y-scope.github.io/yscope-log-viewer/"; +import { LOG_VIEWER_URL } from "src/config/urls";
35-35
: Simplify CSS class assignment.The current implementation uses bracket notation with a fallback to an empty string, which is unnecessarily complex.
- <LinkOutlined className={styles["linkIcon"] || ""}/> + <LinkOutlined className={styles.linkIcon}/>components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx (1)
26-41
: Consider implementing a dark mode variant.The current implementation uses a light theme for syntax highlighting (intellij-light). Consider supporting dark mode for better user experience in different environments.
import SyntaxHighlighter from "react-syntax-highlighter"; import {Typography} from "antd"; +import {useTheme} from 'antd/es/theme/internal'; import LogViewerLink from "./LogViewerLink"; import {highlighterCustomStyles} from "./utils"; import "highlight.js/styles/intellij-light.css"; +import "highlight.js/styles/atom-one-dark.css"; const {Text} = Typography; interface MessageProps { message: string; filePath: string; } /** * Renders a message with syntax highlighting and a file path link. * * @param props * @param props.message * @param props.filePath * @return */ const Message = ({message, filePath}: MessageProps) => { + const { theme } = useTheme(); + const isDarkMode = theme.isDark; return ( <> {/* Parent `Text` component allows syntax highlighter to inherit AntD fonts. */} <Text> <SyntaxHighlighter customStyle={highlighterCustomStyles} language={"armasm"} - useInlineStyles={false} + useInlineStyles={false} + className={isDarkMode ? 'hljs-dark' : 'hljs-light'} >You would also need to add CSS classes to differentiate between light and dark themes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
components/log-viewer-webui/client/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (10)
components/log-viewer-webui/client/package.json
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.module.css
(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.module.css
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/utils.ts
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/index.module.css
(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/index.tsx
(2 hunks)
💤 Files with no reviewable changes (1)
- components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.module.css
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
components/log-viewer-webui/client/src/pages/SearchPage/index.tsx
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/utils.ts
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
🧬 Code Graph Analysis (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/utils.ts (1)
highlighterCustomStyles
(5-11)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (2)
SearchResult
(40-40)searchResultsTableColumns
(41-41)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/index.jsx (1)
SearchResultsTable
(63-186)
🔇 Additional comments (7)
components/log-viewer-webui/client/package.json (1)
23-23
: Dependencies added correctly for syntax highlightingThe added dependencies support the syntax highlighting feature mentioned in the PR objectives, with appropriate version specifications.
Also applies to: 27-27, 33-33
components/log-viewer-webui/client/src/pages/SearchPage/index.module.css (1)
1-6
: Layout styles look goodThe flex container with appropriate spacing and padding creates a clean vertical layout for the search page components.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/utils.ts (1)
1-11
: Well-documented style overridesThe custom styles for the syntax highlighter are appropriately documented with clear reasoning for why they're needed instead of using CSS modules directly.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.module.css (1)
1-3
: Simple and focused stylingThe link icon spacing is appropriate for creating visual separation between the icon and adjacent text.
components/log-viewer-webui/client/src/pages/SearchPage/index.tsx (1)
1-1
: Imports look goodThe necessary CSS module and new component are properly imported.
Also applies to: 3-3
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (1)
1-6
: Clean import statements.The imports look good and properly organized. I like the use of named imports for specific components from the typings module.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
1-15
: SearchResult interface is well defined.The interface properly defines the structure of search results with appropriate types.
// eslint-disable-next-line no-warning-comments | ||
// TODO: Replace with values from database once api implemented. | ||
const DUMMY_RESULTS: SearchResult[] = [ | ||
{ | ||
id: 1, | ||
timestamp: "2023-01-01 12:00:00", | ||
message: "INFO: User login successful for user 'john.doe'.", | ||
filePath: "/var/logs/auth.log", | ||
}, | ||
{ | ||
id: 2, | ||
timestamp: "2023-01-01 12:01:00", | ||
message: "ERROR: Failed to connect to database 'logs_db'.", | ||
filePath: "/var/logs/db.log", | ||
}, | ||
{ | ||
id: 3, | ||
timestamp: "2023-01-01 12:02:00", | ||
message: "WARN: Disk space running low on volume '/var/logs'.", | ||
filePath: "/var/logs/system.log", | ||
}, | ||
{ | ||
id: 4, | ||
timestamp: "2023-01-01 12:03:00", | ||
message: "DEBUG: Processing request ID 12345.", | ||
filePath: "/var/logs/app.log", | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add pagination controls for future implementation.
The dummy data is well structured and provides a good representation of expected search results. While the TODO comment correctly indicates these will be replaced with database values, consider planning for server-side pagination in the component design.
🌐 Web query:
What are the best practices for implementing server-side pagination with Ant Design Table components?
💡 Result:
Best Practices for Implementing Server-Side Pagination with Ant Design Table
Implementing server-side pagination in Ant Design's Table component ensures efficiency and scalability, especially when working with large datasets. Here are the best practices supported by recent examples and the official Ant Design approach:
1. Maintain Pagination State in React
- Use React's
useState
to keep track of pagination details such ascurrent
,pageSize
, andtotal
. - Store the loaded data and loading state in separate state variables for clarity and performance[5][8].
2. Fetch Data Based on Table Pagination Events
- Use the Table's
onChange
event to trigger API calls when the user changes the page, page size, or interacts with sorting/filtering features. - Pass current page and page size to the backend as query parameters.
- Update the data and total count when new data is fetched from the server[4][5][8].
3. Configure the Table's Pagination Prop Properly
- Pass the pagination state to the Table's
pagination
prop:<Table columns={columns} dataSource={data} pagination={{ current: pagination.current, pageSize: pagination.pageSize, total: pagination.total, showSizeChanger: true }} loading={loading} onChange={handleTableChange} />
- Enable
showSizeChanger
for user control over page size options[5][8].
4. Efficient Data Fetching and Loading Feedback
- Show a spinner or loading indicator while data is being fetched by toggling the
loading
prop of the Table[2][5][8]. - Optionally, debounce or throttle API calls if pagination changes rapidly.
5. Always Update the Total Count
- Ensure the total record count (
total
) is updated in your pagination state after each data fetch. This keeps the page count correct and the pagination UI in sync[5][8].
6. Handle Sorting and Filtering
- When using server-side sorting or filtering, capture the additional parameters in
onChange
and pass them to your API. - Backend should handle sorting/filtering and return the paginated, sorted, and/or filtered data[4][10].
7. Robust Error Handling
- Manage and display errors from API failures gracefully, making sure loading states are reset and users are informed if something goes wrong[5][7].
Example Pattern
const [data, setData] = useState([]);
const [pagination, setPagination] = useState({
current: 1,
pageSize: 10,
total: 0
});
const [loading, setLoading] = useState(false);
const fetchData = async (params) => {
setLoading(true);
const response = await api.get('/items', {
params: {
page: params.pagination.current,
size: params.pagination.pageSize
}
});
setData(response.data.items);
setPagination({
...params.pagination,
total: response.data.totalCount
});
setLoading(false);
};
useEffect(() => {
fetchData({ pagination });
}, []);
const handleTableChange = (newPagination, filters, sorter) => {
fetchData({
pagination: newPagination,
filters,
sorter,
});
};
<Table
columns={columns}
dataSource={data}
pagination={pagination}
loading={loading}
onChange={handleTableChange}
/>
[5][8]
Summary Table: Key Practices
Practice | Why It Matters |
---|---|
State management for pagination | Keeps UI and data in sync |
onChange triggers data fetch | Fetches only needed data per page |
Show loading during fetch | Improves user experience |
API returns paginated, total data | Ensures pagination UI is correct |
Error & empty state handling | Prevents UI glitches, informs users |
Support for dynamic page sizes | Enhances flexibility for users |
Pass sorting/filtering to backend | Ensures accurate results |
By following these best practices, you can efficiently implement server-side pagination in Ant Design Table components, resulting in responsive UIs and scalable performance for large data sets[5][8][4].
Citations:
- 1: https://ant.design/components/pagination/
- 2: https://www.youtube.com/watch?v=AXNp_oU1Q80
- 3: https://blog.emiketic.com/2018/implementing-pagination-using-react-redux/
- 4: https://ant.design/components/table/
- 5: https://app.studyraid.com/en/read/12423/401174/table-pagination-handling
- 6: https://docs.uibakery.io/concepts/components/work-with-components/table-server-side-pagination
- 7: https://app.studyraid.com/en/read/12423/401165/pagination-controls
- 8: https://dev.to/jahidulsaeid/how-to-paginated-data-in-ant-design-table-4cne
- 9: https://docs.illacloud.com/server-side-pagination/
- 10: https://www.youtube.com/watch?v=ieXE39FAOU8
Plan for server-side pagination in SearchResultsTable
Once the real API is in place, extend the component to support server-side pagination by:
- Maintaining pagination state (
current
,pageSize
,total
) alongsidedata
andloading
viauseState
. - Configuring the Ant Design
<Table>
’spagination
prop with your pagination state andshowSizeChanger
. - Implementing an
onChange
handler that:
• Receivespagination
,filters
,sorter
• Calls your API withpage
,size
, plus any sorting/filtering params
• Updatesdata
,total
, and resetsloading
- Toggling the Table’s
loading
prop during fetches for clear feedback. - Ensuring the API returns both paginated items and a
totalCount
. - Handling API errors gracefully (reset
loading
, display a message).
File to update:
• components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (around the DUMMY_RESULTS
/TODO block)
/** | ||
* Renders search results in a table. | ||
* | ||
* @return | ||
*/ | ||
const SearchResultsTable = () => { | ||
return ( | ||
<Table<SearchResult> | ||
columns={searchResultsTableColumns} | ||
dataSource={DUMMY_RESULTS} | ||
pagination={false} | ||
rowKey={(record) => record.id.toString()} | ||
virtual={true}/> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance component flexibility with props.
The component is currently hardcoded to use dummy data. Consider enhancing it with props to make it more flexible and reusable:
-const SearchResultsTable = () => {
+interface SearchResultsTableProps {
+ dataSource?: SearchResult[];
+ loading?: boolean;
+ onSortChange?: (field: string, order: 'ascend' | 'descend' | undefined) => void;
+}
+
+const SearchResultsTable = ({
+ dataSource = DUMMY_RESULTS,
+ loading = false,
+ onSortChange,
+}: SearchResultsTableProps) => {
return (
<Table<SearchResult>
columns={searchResultsTableColumns}
- dataSource={DUMMY_RESULTS}
+ dataSource={dataSource}
+ loading={loading}
+ onChange={(pagination, filters, sorter) => {
+ if (onSortChange && !Array.isArray(sorter)) {
+ onSortChange(sorter.field as string, sorter.order);
+ }
+ }}
pagination={false}
rowKey={(record) => record.id.toString()}
virtual={true}/>
);
};
📝 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.
/** | |
* Renders search results in a table. | |
* | |
* @return | |
*/ | |
const SearchResultsTable = () => { | |
return ( | |
<Table<SearchResult> | |
columns={searchResultsTableColumns} | |
dataSource={DUMMY_RESULTS} | |
pagination={false} | |
rowKey={(record) => record.id.toString()} | |
virtual={true}/> | |
); | |
}; | |
/** | |
* Renders search results in a table. | |
* | |
* @return | |
*/ | |
interface SearchResultsTableProps { | |
dataSource?: SearchResult[]; | |
loading?: boolean; | |
onSortChange?: (field: string, order: 'ascend' | 'descend' | undefined) => void; | |
} | |
const SearchResultsTable = ({ | |
dataSource = DUMMY_RESULTS, | |
loading = false, | |
onSortChange, | |
}: SearchResultsTableProps) => { | |
return ( | |
<Table<SearchResult> | |
columns={searchResultsTableColumns} | |
dataSource={dataSource} | |
loading={loading} | |
onChange={(pagination, filters, sorter) => { | |
if (onSortChange && !Array.isArray(sorter)) { | |
onSortChange(sorter.field as string, sorter.order); | |
} | |
}} | |
pagination={false} | |
rowKey={(record) => record.id.toString()} | |
virtual={true} | |
/> | |
); | |
}; |
<Text> | ||
<SyntaxHighlighter | ||
customStyle={highlighterCustomStyles} | ||
language={"armasm"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider using more appropriate language for log syntax highlighting.
The current implementation uses "armasm" as the language for syntax highlighting, which seems unusual for log messages. Consider using a more appropriate language like "log", "text" or "plaintext".
- language={"armasm"}
+ language={"log"}
🌐 Web query:
What's the best syntax highlighting language to use for log files in react-syntax-highlighter?
💡 Result:
React Syntax Highlighter offers a wide range of language support for syntax highlighting in React applications, but there isn't a specific language explicitly designed for log files. However, several options can be effective for highlighting log files depending on your specific log format.
Best Options for Log File Syntax Highlighting
For log files in react-syntax-highlighter, these languages work particularly well:
Plain Text
The simplest approach is to use "text" as your language option, which will render your logs in monospaced font without specific highlighting[1][7]. This works well for simple logs where you don't need complex syntax highlighting.
<SyntaxHighlighter language="text" style={solarizedlight}>
{logFileContent}
</SyntaxHighlighter>
Shell/Bash
For command-line or terminal-style logs, the "shell" or "bash" language options provide useful highlighting that can make timestamps, errors, and command outputs more distinguishable[10]. These languages highlight typical patterns found in terminal outputs.
HTTP
If your logs contain HTTP requests and responses, the "http" language option can be particularly useful as it will highlight status codes, headers, and request methods[10].
JavaScript Console
For logs that originate from browser consoles or JavaScript applications, using "javascript" as the language can provide good highlighting for objects, errors, and various data types[5][6].
Customization Options
Regardless of which language you choose, react-syntax-highlighter offers several customization features that can enhance log file readability:
Line Numbers
Adding line numbers helps with referencing specific parts of lengthy logs[5]:
<SyntaxHighlighter
language="text"
style={solarizedlight}
showLineNumbers>
{logFileContent}
</SyntaxHighlighter>
Highlighting Specific Lines
You can highlight important lines (like errors) in your logs using the wrapLines
and lineProps
options[5]:
const lineProps = (lineNumber) => {
let style = { display: 'block' };
if (lineNumber === 2) { // Highlight line containing error
style.backgroundColor = '#ffdd57';
}
return { style };
};
<SyntaxHighlighter
language="text"
style={solarizedlight}
wrapLines
lineProps={lineProps}>
{logFileContent}
</SyntaxHighlighter>
Custom Styling
You can further customize the appearance using the customStyle
prop to match your application's theme[1][5].
If your log files have a specific format or structure, you might need to experiment with different language options to find the one that best highlights the important parts of your logs. In some cases, you might even consider creating a custom language definition if your logs have a very specific structure that isn't well-served by the existing options.
Citations:
- 1: https://github.com/react-syntax-highlighter/react-syntax-highlighter
- 2: https://github.com/react-syntax-highlighter
- 3: https://react-syntax-highlighter.github.io/react-syntax-highlighter/demo/diff.html
- 4: https://classic.yarnpkg.com/en/package/react-syntax-highlighter
- 5: https://www.dhiwise.com/post/crafting-beautiful-code-blocks-with-react-syntax-highlighter
- 6: https://www.commoninja.com/blog/build-code-syntax-highlighter-react
- 7: https://mdxjs.com/guides/syntax-highlighting/
- 8: https://blog.logrocket.com/exploring-best-syntax-highlighting-libraries/
- 9: https://www.npmjs.com/package/@types/react-syntax-highlighter
- 10: https://github.com/react-syntax-highlighter/react-syntax-highlighter/blob/master/AVAILABLE_LANGUAGES_PRISM.MD
Switch to Plain Text Highlighting for Logs
To improve readability of log output, use the plain text highlighter instead of “armasm”. For example:
• File:
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx
Line: 33
- language={"armasm"}
+ language="text"
If your logs include shell commands, HTTP messages or JavaScript console output, consider using "bash"
, "http"
or "javascript"
respectively.
📝 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.
language={"armasm"} | |
language="text" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise, everything looks good. The syntax highlighting works nicely!
Sorting doesn’t seem to work as expected. See the comments below.
{ | ||
dataIndex: "timestamp", | ||
key: "timestamp", | ||
sorter: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be missing something, sorting doesn't seem to work. According to the docs, it looks like sorter
is expected to be a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sorter will be server side, like it will need to launch a new query to the backend, so this is just a dummy sorter until database api is implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sorter will be server side
It makes sense to set the sorter
as true. I believe we will need to register a onChange
handler on the Table to detect if any sorter changes (i.e., user requests a different sorting order on a column), right?
(I haven't run the code locally yet. Do the sorting indicators (asc / desc) change as we click the table headers?)
We can register the handler and print a log if the sorter changes, though this will be just boilerplate code until we submit the API integration PR. @davemarco I'll let you decide whether you want to add the handler in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense
(I haven't run the code locally yet. Do the sorting indicators (asc / desc) change as we click the table headers?)
The indicator changes.
{message} | ||
</SyntaxHighlighter> | ||
</Text> | ||
<LogViewerLink filePath={filePath}/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aesthetics: it might look cleaner to display the file link in a separate column. Users could also filter or sort based on the file name.
If horizontal space is a concern, we could consider adding buttons to show/hide columns later.
(I don't have full context of the PR, so feel push back)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, this is what the old webui did. I do agree the columns are better, but I think this will be later once we setup dynamic columns for all fields in the query results(clp-s only). for now i think better/simpler to match old ui.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, I believe we can add a right pop-up panel (a.k.a. a "drawer") for displaying metadata of a specific log event when selected.
For now, since we intend to make the log viewer links visible (as a feature highlight), it makes sense to leave the links directly in the table. Adding an extra column for file name display would work, but as you mention we could be leaving a lot of gap in the log viewer link cells when the message is too long - it could be a waste of space.
Therefore, for now I believe it is fine to leave the links at the bottom of the message cells.
@hoophalab can i start reviewing this, or do you want to take a further look? |
Description
Adds a table which holds search results. Data is currently dummy data.
Syntax highlighting is provided by both react-syntax-highlighter and highlighter.js.
react-syntax-highligher is a react wrapper of highlighter.js. However it dosent have all the themes from highlighter.js, so
I imported highlighter as well to get intelli-light theme.
Reminders for when server implemented:
fancier features like server side filtering are not yet implemented. This will have to wait until server is working.
I took a look at table rendering code in old webui, and there is a fancy sensor to load more results. While its cute, I think we can drop this in new webui as it there are max 1000 results anyways, so I assume the sensor probably has little performance benefit. I also looked at another observability tool as well, and it appears they dont use a sensor to request more results.
Checklist
breaking change.
Validation performed
Table looks okay.
Summary by CodeRabbit