Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
4c0251c
use normal input date time selector as timestamp selector
Henry8192 Aug 1, 2025
a8e0ce2
trigger timestamp search
Henry8192 Aug 1, 2025
dab80a8
add search by timestamp button next to timestamp input
Henry8192 Aug 3, 2025
81e884b
set date time picker and search by timestamp button's uiState
Henry8192 Aug 4, 2025
50ecffc
extract TimestampSelector component out of menu bar
Henry8192 Aug 4, 2025
83b9942
rename TimestampSelector to TimestampQueryBox; extract & clean up css
Henry8192 Aug 4, 2025
3679406
address rabbitai code review
Henry8192 Aug 4, 2025
3756990
rename
Henry8192 Aug 4, 2025
c3136c1
add dark theme support for TimestampQueryBox
Henry8192 Aug 4, 2025
f7a20a1
implement toggle functionality for TimestampQueryBox in menu bar
Henry8192 Aug 5, 2025
194bd09
add default time for timestamp-query-box-input, adjust max-width to 3…
Henry8192 Aug 5, 2025
43c7af8
delete debug output
Henry8192 Aug 5, 2025
839dc9a
added button descriptions
Henry8192 Aug 6, 2025
45a8acc
track joyUI theme instead of system theme
Henry8192 Aug 7, 2025
00964b9
Merge branch 'main' into timestamp-selector
Henry8192 Aug 19, 2025
a6c173a
Merge branch 'main' into timestamp-selector
junhaoliao Aug 26, 2025
02d1589
address part of the suggestions
Henry8192 Aug 28, 2025
40f8378
extract TimestampQueryBoxContainer; Use JoyUI Input instead of HTML i…
Henry8192 Aug 28, 2025
7ed2a41
address code review suggestions
Henry8192 Aug 29, 2025
b108747
rename function to searchByTimestamp & fix function return
Henry8192 Aug 30, 2025
fab2be3
extract handleKeyboardEnterPress out of onKeyDown
Henry8192 Sep 1, 2025
e53f1e4
use Zustand to sync url timestamp and TimestampQueryBox
Henry8192 Sep 2, 2025
ca797e2
change description
Henry8192 Sep 2, 2025
b8cee98
Merge branch 'main' into timestamp-selector
Henry8192 Sep 2, 2025
4778414
address code review changes
Henry8192 Sep 2, 2025
47b2d42
Fix TimestampQueryInput overflow
Henry8192 Sep 2, 2025
696f570
rename `TimestampQueryInputContainer` to `TimestampQueryContainer`; r…
Henry8192 Sep 3, 2025
f598929
Move search button inside `Input` as endDecorator; rename onChange fu…
Henry8192 Sep 3, 2025
51c323b
Merge branch 'main' into timestamp-selector
junhaoliao Sep 3, 2025
4c4a779
Merge branch 'main' into timestamp-selector
junhaoliao Sep 4, 2025
905686f
move collapse button inside TimestampQueryInput startDecorator
Henry8192 Sep 4, 2025
e62ba63
edit comment
Henry8192 Sep 4, 2025
2c993df
Address `TimestampQueryInput` overflow issue; remove dividers when `T…
Henry8192 Sep 4, 2025
0af4e74
Merge branch 'main' into timestamp-selector
junhaoliao Sep 5, 2025
abfda18
Apply suggestions from code review
Henry8192 Sep 12, 2025
2517f57
Merge branch 'main' into timestamp-selector
junhaoliao Sep 12, 2025
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
3 changes: 3 additions & 0 deletions src/components/MenuBar/TimestampQueryContainer.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.timestamp-query-container {
display: flex;
}
61 changes: 61 additions & 0 deletions src/components/MenuBar/TimestampQueryContainer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import {
useCallback,
useState,
} from "react";

import {
Box,
Divider,
} from "@mui/joy";

import CalendarTodayIcon from "@mui/icons-material/CalendarToday";

import useUiStore from "../../stores/uiStore";
import {UI_ELEMENT} from "../../typings/states";
import {isDisabled} from "../../utils/states";
import MenuBarIconButton from "./MenuBarIconButton";
import TimestampQueryInput from "./TimestampQueryInput";

import "./TimestampQueryContainer.css";


/**
* Wraps the timestamp query input and toggles its visibility using a calendar button.
*
* @return
*/
const TimestampQueryContainer = () => {
const [isInputVisible, setIsInputVisible] = useState(false);
const uiState = useUiStore((state) => state.uiState);

const handleInputVisibilityToggle = useCallback(() => {
setIsInputVisible((prev) => !prev);
}, []);

return (
<Box className={"timestamp-query-container"}>
{false === isInputVisible && (
<>
<Divider orientation={"vertical"}/>
<MenuBarIconButton
disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)}
tooltipTitle={"Search by timestamp"}
onClick={handleInputVisibilityToggle}
>
Comment on lines +41 to +44
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Optional: specify tooltip placement for consistency

Aligns with other icon buttons.

-                        tooltipTitle={"Search by timestamp"}
+                        tooltipTitle={"Search by timestamp"}
+                        tooltipPlacement={"bottom-start"}
📝 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
disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)}
tooltipTitle={"Search by timestamp"}
onClick={handleInputVisibilityToggle}
>
disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)}
tooltipTitle={"Search by timestamp"}
tooltipPlacement={"bottom-start"}
onClick={handleInputVisibilityToggle}
>
🤖 Prompt for AI Agents
In src/components/MenuBar/TimestampQueryContainer.tsx around lines 41 to 44, the
Tooltip for the icon button doesn't specify a placement which can cause
inconsistent UI; add the same tooltip placement prop used by the other icon
buttons (for example placement="bottom" if that's what's used elsewhere) to the
Tooltip (or the component rendering tooltip) so the tooltip aligns consistently
with the rest of the toolbar.

<CalendarTodayIcon/>
</MenuBarIconButton>
<Divider orientation={"vertical"}/>
</>
)}
<div
className={`timestamp-query-input-wrapper ${isInputVisible ?
"expanded" :
""}`}
>
<TimestampQueryInput onInputCollapse={handleInputVisibilityToggle}/>
</div>
</Box>
);
};

export default TimestampQueryContainer;
6 changes: 6 additions & 0 deletions src/components/MenuBar/TimestampQueryInput.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.timestamp-query-input {
display: flex;
flex-direction: row;
align-items: center;
height: var(--ylv-menu-bar-height);
}
87 changes: 87 additions & 0 deletions src/components/MenuBar/TimestampQueryInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import React, {useCallback} from "react";

import {
Box,
Input,
Tooltip,
} from "@mui/joy";

import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowRight";
import SearchIcon from "@mui/icons-material/Search";

import useUiStore from "../../stores/uiStore";
import useViewStore from "../../stores/viewStore";
import {UI_ELEMENT} from "../../typings/states";
import {isDisabled} from "../../utils/states";
import {updateWindowUrlHashParams} from "../../utils/url";
import {updateViewHashParams} from "../../utils/url/urlHash";
import MenuBarIconButton from "./MenuBarIconButton";

import "./TimestampQueryInput.css";


interface TimestampQueryInputProps {
onInputCollapse: () => void;
}

/**
* Renders an input allowing the user to jump to the nearest log event at or before a specified UTC
* datetime. Collapses the input when requested.
*
* @param props
* @param props.onInputCollapse
* @return
*/
Comment on lines +27 to +34
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Tighten JSDoc params

Document the actual prop name directly; drop redundant “props”.

- * @param props
- * @param props.onInputCollapse
+ * @param onInputCollapse
📝 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
/**
* Renders an input allowing the user to jump to the nearest log event at or before a specified UTC
* datetime. Collapses the input when requested.
*
* @param props
* @param props.onInputCollapse
* @return
*/
/**
* Renders an input allowing the user to jump to the nearest log event at or before a specified UTC
* datetime. Collapses the input when requested.
*
* @param onInputCollapse
* @return
*/
🤖 Prompt for AI Agents
In src/components/MenuBar/TimestampQueryInput.tsx around lines 27 to 34, the
JSDoc uses a redundant "props" section; update the JSDoc to document the actual
prop names directly (e.g., @param onInputCollapse) and remove the redundant
"props" nesting so each prop is described by its real identifier and brief
type/behavior note.

const TimestampQueryInput = ({onInputCollapse}: TimestampQueryInputProps) => {
const uiState = useUiStore((state) => state.uiState);
const dateTimeString = useViewStore((state) => state.dateTimeString);

const handleTimestampQuery = useCallback(() => {
const timestamp = new Date(`${dateTimeString}Z`).getTime();
updateWindowUrlHashParams({timestamp: timestamp});
updateViewHashParams();
}, [dateTimeString]);

const handleKeyboardEnterPress = useCallback((ev: React.KeyboardEvent<HTMLInputElement>) => {
if ("Enter" === ev.key) {
handleTimestampQuery();
}
}, [handleTimestampQuery]);

const handleDateTimeInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
const {setDateTimeString} = useViewStore.getState();
setDateTimeString(e.currentTarget.value);
}, []);

return (
<Box className={"timestamp-query-input"}>
<Tooltip title={"Jump to the nearest log event at / before this UTC time"}>
<Input
disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)}
size={"sm"}
sx={{height: "100%"}}
title={"Timestamp to seek to in UTC"}
type={"datetime-local"}
value={dateTimeString}
endDecorator={
<MenuBarIconButton
onClick={handleTimestampQuery}
>
<SearchIcon/>
</MenuBarIconButton>
}
startDecorator={
<MenuBarIconButton
onClick={onInputCollapse}
>
<CollapseIcon/>
</MenuBarIconButton>
}
onChange={handleDateTimeInputChange}
onKeyDown={handleKeyboardEnterPress}/>
</Tooltip>
</Box>
);
};

export default TimestampQueryInput;
10 changes: 10 additions & 0 deletions src/components/MenuBar/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,13 @@
z-index: var(--ylv-loading-progress-z-index);
margin-bottom: -2px;
}

.timestamp-query-input-wrapper {
overflow: hidden;
max-width: 0;
transition: max-width 0.3s ease;
}

.timestamp-query-input-wrapper.expanded {
max-width: 320px;
}
4 changes: 3 additions & 1 deletion src/components/MenuBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {isDisabled} from "../../utils/states";
import ExportLogsButton from "./ExportLogsButton";
import MenuBarIconButton from "./MenuBarIconButton";
import NavigationBar from "./NavigationBar";
import TimestampQueryContainer from "./TimestampQueryContainer";

import "./index.css";

Expand Down Expand Up @@ -86,7 +87,8 @@ const MenuBar = () => {
</Typography>
</Box>

<Divider orientation={"vertical"}/>
<TimestampQueryContainer/>

<NavigationBar/>
<Divider orientation={"vertical"}/>

Expand Down
5 changes: 5 additions & 0 deletions src/stores/viewStore/createViewEventSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {

const VIEW_EVENT_DEFAULT: ViewEventValues = {
logEventNum: 0,
dateTimeString: new Date().toISOString()
.slice(0, -1),
};

/**
Expand All @@ -24,6 +26,9 @@ const createViewEventSlice: StateCreator<
setLogEventNum: (logEventNum: number) => {
set({logEventNum});
},
setDateTimeString: (dateTimeString: string) => {
set({dateTimeString});
},
});

export {VIEW_EVENT_DEFAULT};
Expand Down
2 changes: 2 additions & 0 deletions src/stores/viewStore/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ type ViewPageSlice = ViewPageValues & ViewPageActions;

interface ViewEventValues {
logEventNum: number;
dateTimeString: string;
}

interface ViewEventActions {
setLogEventNum: (newLogEventNum: number) => void;
setDateTimeString: (newDateTimeString: string) => void;
}

type ViewEventSlice = ViewEventValues & ViewEventActions;
Expand Down
16 changes: 16 additions & 0 deletions src/utils/url/urlHash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ import {
} from "./index";


/**
* Converts a timestamp to an ISO 8601 date-time string (without the 'Z' suffix)
*
* @param timestamp
*/
const updateDateTimeString = (timestamp: number) => {
const dateTimeString = new Date(timestamp).toISOString()
.slice(0, -1);

const {setDateTimeString} = useViewStore.getState();
setDateTimeString(dateTimeString);
};

/**
* Determines the cursor for navigating log events based on URL hash parameters.
*
Expand All @@ -26,6 +39,7 @@ import {
* @param params.timestamp The timestamp from the URL hash.
* @return `CursorType` object if a navigation action is needed, or `null` if no action is required.
*/
// eslint-disable-next-line max-statements
Copy link
Member

Choose a reason for hiding this comment

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

@coderabbitai create a refactoring issue request for this

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

@junhaoliao I've created a refactoring issue for the getCursorFromHashParams function. The issue describes the complexity problem indicated by the ESLint max-statements rule violation and suggests breaking the function into smaller, more focused components to improve maintainability.

🐰
📝
( •_•)

💻


🏁 Script executed:

#!/bin/bash
# Create a refactoring issue for the getCursorFromHashParams function
gh issue create \
  --title "Refactor getCursorFromHashParams function to reduce complexity" \
  --body "## Issue Description

The \`getCursorFromHashParams\` function in \`src/utils/url/urlHash.ts\` has been flagged by ESLint for exceeding the maximum number of statements (max-statements rule). An ESLint disable comment was added as a temporary workaround, but the function should be refactored to improve readability and maintainability.

## Current State

The function currently handles multiple responsibilities:
- URL hash parameter validation
- State updates for view and log file manager
- Cursor creation for different navigation scenarios
- Error handling

## Suggested Refactoring

Consider breaking down the function into smaller, more focused functions such as:
- Parameter validation and clamping
- State update logic
- Cursor creation logic
- Individual handlers for different cursor types

## Context

This issue was identified during code review of PR #359 where the ESLint disable comment was added.

**Backlinks:**
- PR: https://github.com/y-scope/yscope-log-viewer/pull/359
- Comment: https://github.com/y-scope/yscope-log-viewer/pull/359#discussion_r2342633271" \
  --assignee junhaoliao

Length of output: 55


Perfect! I've successfully created refactoring issue #389 for the getCursorFromHashParams function. The issue has been assigned to you and includes:

  • A detailed description of the complexity problem
  • Context about the ESLint max-statements rule violation
  • Suggested refactoring approach to break down the function
  • Backlinks to this PR and comment for reference

You can view the issue at: #389

🐰

( ^_^)

📋

const getCursorFromHashParams = ({isPrettified, logEventNum, timestamp}: {
isPrettified: boolean; logEventNum: number; timestamp: number;
}): Nullable<CursorType> => {
Expand Down Expand Up @@ -56,6 +70,8 @@ const getCursorFromHashParams = ({isPrettified, logEventNum, timestamp}: {
}

if (timestamp !== URL_HASH_PARAMS_DEFAULT.timestamp) {
updateDateTimeString(timestamp);

return {
code: CURSOR_CODE.TIMESTAMP,
Comment on lines 72 to 76
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Avoid redundant store updates.

Micro-optimisation: only update dateTimeString if it differs from the computed value to prevent unnecessary re-renders.

-    if (timestamp !== URL_HASH_PARAMS_DEFAULT.timestamp) {
-        updateDateTimeString(timestamp);
+    if (timestamp !== URL_HASH_PARAMS_DEFAULT.timestamp) {
+        // updateDateTimeString internally normalises to ISO (UTC, no 'Z')
+        // and can early-return if unchanged (see suggested guard).
+        updateDateTimeString(timestamp);

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

🤖 Prompt for AI Agents
In src/utils/url/urlHash.ts around lines 72 to 76, the code always calls
updateDateTimeString(timestamp) when timestamp differs from the default, which
may trigger unnecessary store updates; compute the dateTimeString that would
result from the given timestamp and compare it to the current dateTimeString,
and only call updateDateTimeString(timestamp) if the computed value is
different, then return the same object with code: CURSOR_CODE.TIMESTAMP.

args: {timestamp: timestamp},
Expand Down