Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 32 additions & 2 deletions components/log-viewer-webui/client/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion components/log-viewer-webui/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"axios": "^1.7.9",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"react-router": "^7.4.1"
"react-router": "^7.4.1",
"zustand": "^5.0.3"
},
"devDependencies": {
"@types/react": "^19.0.10",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import {SearchOutlined} from "@ant-design/icons";
import {
Button,
Tooltip,
} from "antd";

import useSearchStore, {SEARCH_STATE_DEFAULT} from "../SearchState";
import styles from "./index.module.css";


/**
* Renders a button to submit the search query.
*
* @return
*/
const SearchButton = () => {
const queryString = useSearchStore((state) => state.queryString);

const isQueryStringEmpty: boolean =
queryString === SEARCH_STATE_DEFAULT.queryString;

return (
<Tooltip
title={isQueryStringEmpty ?
"Enter query to search" :
""}
>
<Button
className={styles["gradientButton"] || ""}
disabled={isQueryStringEmpty}
icon={<SearchOutlined/>}
size={"large"}
type={"primary"}
>
Search
</Button>
</Tooltip>
);
};

Comment thread
davemarco marked this conversation as resolved.

export default SearchButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.timeRangeInputContainer {
display: inline-flex;
}

/* Makes border flush with range picker */
.customSelected :global(.ant-select-selector) {
border-top-right-radius: 0 !important;
border-bottom-right-radius: 0 !important;
}

.rangePicker {
border-top-left-radius: 0;
border-bottom-left-radius: 0;
min-width: 200px;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import {useState} from "react";

import {
DatePicker,
Select,
} from "antd";
import dayjs from "dayjs";

import useSearchStore from "../../SearchState";
import styles from "./index.module.css";
import {
DEFAULT_TIME_RANGE,
isValidDateRange,
TIME_RANGE_OPTION,
TIME_RANGE_OPTION_DAYJS_MAP,
TIME_RANGE_OPTION_NAMES,
} from "./utils";


/**
* Renders controls for selecting a time range for queries. By default, the component is
* a select dropdown with a list of preset time ranges. If the user selects "Custom",
* a date range picker is also displayed.
*
* @return
*/
const TimeRangeInput = () => {
const updateTimeRange = useSearchStore((state) => state.updateTimeRange);
const [selectedOption, setSelectedOption] = useState<TIME_RANGE_OPTION>(DEFAULT_TIME_RANGE);

const handleSelectChange = (timeRangeOption: TIME_RANGE_OPTION) => {
setSelectedOption(timeRangeOption);
if (timeRangeOption !== TIME_RANGE_OPTION.CUSTOM) {
const dayJsRange = TIME_RANGE_OPTION_DAYJS_MAP[timeRangeOption];
updateTimeRange(dayJsRange);
}
};

const handleRangePickerChange = (
dates: [dayjs.Dayjs | null, dayjs.Dayjs | null] | null
) => {
if (!isValidDateRange(dates)) {
return;
}
updateTimeRange(dates);
};

return (
<div
className={styles["timeRangeInputContainer"]}
>
<Select
Comment thread
davemarco marked this conversation as resolved.
defaultValue={DEFAULT_TIME_RANGE}
listHeight={300}
options={TIME_RANGE_OPTION_NAMES.map((option) => ({label: option, value: option}))}
popupMatchSelectWidth={false}
size={"large"}
variant={"filled"}
className={selectedOption === TIME_RANGE_OPTION.CUSTOM ?
(styles["customSelected"] || "") :
""}
onChange={handleSelectChange}/>
{selectedOption === TIME_RANGE_OPTION.CUSTOM && (
<DatePicker.RangePicker
className={styles["rangePicker"] || ""}
showNow={true}
showTime={true}
size={"large"}
onChange={(dates) => {
handleRangePickerChange(dates);
}}/>
)}
</div>
);
};

Comment thread
davemarco marked this conversation as resolved.

export default TimeRangeInput;
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import dayjs from "dayjs";


/**
* Time range options.
*/
enum TIME_RANGE_OPTION {
LAST_15_MINUTES = "Last 15 Minutes",
LAST_HOUR = "Last Hour",
TODAY = "Today",
YESTERDAY = "Yesterday",
LAST_7_DAYS = "Last 7 Days",
LAST_30_DAYS = "Last 30 Days",
MONTH_TO_DATE = "Month to Date",
CUSTOM = "Custom",
}

const DEFAULT_TIME_RANGE = TIME_RANGE_OPTION.TODAY;

/* eslint-disable no-magic-numbers */
const TIME_RANGE_OPTION_DAYJS_MAP: Record<TIME_RANGE_OPTION, [dayjs.Dayjs, dayjs.Dayjs]> = {
[TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(15, "minute"),
dayjs()],
[TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(1, "hour"),
dayjs()],
[TIME_RANGE_OPTION.TODAY]: [dayjs().startOf("day"),
dayjs().endOf("day")],
[TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(1, "d"),
dayjs().subtract(1, "d")],
[TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(7, "d"),
dayjs()],
[TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(30, "d"),
dayjs()],
[TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs().startOf("month"),
dayjs()],

// Custom option is just a placeholder for typing purposes, its DayJs values should not
// be used.
[TIME_RANGE_OPTION.CUSTOM]: [dayjs(),
dayjs()],
};
Comment on lines +20 to +41

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove eslint disabling and use constants for magic numbers

Instead of disabling the eslint rule, define constants for these values to improve code maintainability.

- /* eslint-disable no-magic-numbers */
+ // Time constants in minutes/hours/days
+ const FIFTEEN_MINUTES = 15;
+ const ONE_HOUR = 1;
+ const ONE_DAY = 1;
+ const SEVEN_DAYS = 7;
+ const THIRTY_DAYS = 30;

const TIME_RANGE_OPTION_DAYJS_MAP: Record<TIME_RANGE_OPTION, [dayjs.Dayjs, dayjs.Dayjs]> = {
-   [TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(15, "minute"),
+   [TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(FIFTEEN_MINUTES, "minute"),
        dayjs()],
-   [TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(1, "hour"),
+   [TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(ONE_HOUR, "hour"),
        dayjs()],
    [TIME_RANGE_OPTION.TODAY]: [dayjs().startOf("day"),
        dayjs().endOf("day")],
-   [TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(1, "d"),
-       dayjs().subtract(1, "d")],
+   [TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(ONE_DAY, "d").startOf("day"),
+       dayjs().subtract(ONE_DAY, "d").endOf("day")],
-   [TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(7, "d"),
+   [TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(SEVEN_DAYS, "d"),
        dayjs()],
-   [TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(30, "d"),
+   [TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(THIRTY_DAYS, "d"),
        dayjs()],
    [TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs().startOf("month"),
        dayjs()],
📝 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
/* eslint-disable no-magic-numbers */
const TIME_RANGE_OPTION_DAYJS_MAP: Record<TIME_RANGE_OPTION, [dayjs.Dayjs, dayjs.Dayjs]> = {
[TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(15, "minute"),
dayjs()],
[TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(1, "hour"),
dayjs()],
[TIME_RANGE_OPTION.TODAY]: [dayjs().startOf("day"),
dayjs().endOf("day")],
[TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(1, "d"),
dayjs().subtract(1, "d")],
[TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(7, "d"),
dayjs()],
[TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(30, "d"),
dayjs()],
[TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs().startOf("month"),
dayjs()],
// Custom option is just a placeholder for typing purposes, its DayJs values should not
// be used.
[TIME_RANGE_OPTION.CUSTOM]: [dayjs(),
dayjs()],
};
// Time constants in minutes/hours/days
const FIFTEEN_MINUTES = 15;
const ONE_HOUR = 1;
const ONE_DAY = 1;
const SEVEN_DAYS = 7;
const THIRTY_DAYS = 30;
const TIME_RANGE_OPTION_DAYJS_MAP: Record<TIME_RANGE_OPTION, [dayjs.Dayjs, dayjs.Dayjs]> = {
[TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(FIFTEEN_MINUTES, "minute"),
dayjs()],
[TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(ONE_HOUR, "hour"),
dayjs()],
[TIME_RANGE_OPTION.TODAY]: [dayjs().startOf("day"),
dayjs().endOf("day")],
[TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(ONE_DAY, "d").startOf("day"),
dayjs().subtract(ONE_DAY, "d").endOf("day")],
[TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(SEVEN_DAYS, "d"),
dayjs()],
[TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(THIRTY_DAYS, "d"),
dayjs()],
[TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs().startOf("month"),
dayjs()],
// Custom option is just a placeholder for typing purposes, its DayJs values should not
// be used.
[TIME_RANGE_OPTION.CUSTOM]: [dayjs(),
dayjs()],
};



/**
* Key names in enum `TIME_RANGE_OPTION`.
*/
const TIME_RANGE_OPTION_NAMES = Object.freeze(
Object.values(TIME_RANGE_OPTION).filter((value) => "string" === typeof value)
);

/**
* Validates dates provided by the range picker callback are non-null.
*
* @param dates
* @return
*/
const isValidDateRange = (
dates: [dayjs.Dayjs | null, dayjs.Dayjs | null] | null
): dates is [dayjs.Dayjs, dayjs.Dayjs] => {
return null !== dates && null !== dates[0] && null !== dates[1];
};

Comment thread
davemarco marked this conversation as resolved.

export {
DEFAULT_TIME_RANGE,
isValidDateRange,
TIME_RANGE_OPTION,
TIME_RANGE_OPTION_DAYJS_MAP,
TIME_RANGE_OPTION_NAMES,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.searchControlsContainer {
margin-top: 20px;
padding: 0 20px;
display: flex;
gap: 10px;
}

.gradientButton {
background-image: linear-gradient(135deg, #6253e1, #04befe);
color: white;
transition: background 0.3s;
background-size: 200% auto;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {Input} from "antd";

import useSearchStore from "../SearchState";
import styles from "./index.module.css";
import SearchButton from "./SearchButton";
import TimeRangeInput from "./TimeRangeInput";


/**
* Renders controls for submitting queries.
*
* @return
*/
const SearchControls = () => {
const queryString = useSearchStore((state) => state.queryString);
const updateQueryString = useSearchStore((state) => state.updateQueryString);

return (
<div className={styles["searchControlsContainer"]}>
<Input
placeholder={"Enter your query"}
size={"large"}
value={queryString}
onChange={(e) => {
updateQueryString(e.target.value);
}}/>
<TimeRangeInput/>
<SearchButton/>
</div>
);
};

Comment thread
davemarco marked this conversation as resolved.

export default SearchControls;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this looks very clean now. nice!

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import dayjs from "dayjs";
import {create} from "zustand";

import {
DEFAULT_TIME_RANGE,
TIME_RANGE_OPTION_DAYJS_MAP,
} from "./SearchControls/TimeRangeInput/utils";


/**
* Default values of the search state.
*/
const SEARCH_STATE_DEFAULT = Object.freeze({
queryString: "",
timeRange: TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE],
});

interface SearchState {
queryString: string;
timeRange: [dayjs.Dayjs, dayjs.Dayjs];
updateQueryString: (query: string) => void;
updateTimeRange: (range: [dayjs.Dayjs, dayjs.Dayjs]) => void;
}

const useSearchStore = create<SearchState>((set) => ({
queryString: SEARCH_STATE_DEFAULT.queryString,
timeRange: SEARCH_STATE_DEFAULT.timeRange,
updateQueryString: (query) => {
set({queryString: query});
},
updateTimeRange: (range) => {
set({timeRange: range});
},
}));

Comment thread
davemarco marked this conversation as resolved.

export {SEARCH_STATE_DEFAULT};
export default useSearchStore;
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import SearchControls from "./SearchControls";


/**
* Provides a search interface that allows users to query archives and visualize search results.
*
* @return
*/
const SearchPage = () => {
return (
<div>
<h1>Search Page</h1>
<p>This is the Search Page.</p>
</div>
<SearchControls/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be great if we could make SearchControls a reusable component and manage it through props, similar to mui's Select (code).

We could have queryString, timeRange, onQueryStringChange, onTimeRangeChange, and onSearchClicked.

But of course, whether making this change worth the time depends on your opinion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if we're to reuse the <SearchControls/> component, the suggestion makes sense

before we proceed, do we see where we will be reusing the component? i.e., is there any other page / place we will be using this?

);
};

Comment thread
davemarco marked this conversation as resolved.
Expand Down