feat(MenuBar): Add timestamp query input for jumping to log events by UTC timestamp (resolves #118). #359
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a toggleable timestamp search UI to the MenuBar (toggle button + UTC datetime-local input), new CSS for a collapsible input, a view-store Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant MB as MenuBar
participant TQC as TimestampQueryContainer
participant TQI as TimestampQueryInput
participant VS as ViewStore
participant URL as URL Hash
U->>MB: Click "Search by timestamp" toggle
MB->>TQC: Toggle expanded state
TQC->>TQI: Render input when expanded
U->>TQI: Edit datetime-local
TQI->>VS: setDateTimeString(newValue)
alt Submit via Enter or Search button
TQI->>URL: Update hash with timestamp (UTC ms)
URL-->>MB: Hashchange processed (navigation)
end
sequenceDiagram
autonumber
participant URL as URL Hash Parser
participant VS as ViewStore
participant NAV as Navigation
URL->>URL: getCursorFromHashParams()
alt timestamp present (non-default)
URL->>VS: setDateTimeString(ISO string without trailing Z)
URL->>NAV: Return TIMESTAMP cursor
else no timestamp
URL->>NAV: Return existing/default cursor
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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. Comment |
Henry8192
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/components/MenuBar/TimestampQueryBox.css(1 hunks)src/components/MenuBar/TimestampQueryBox.tsx(1 hunks)src/components/MenuBar/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
📚 Learning: the search button in the `menubar` component (`new-log-viewer/src/components/menubar/index.tsx`) is ...
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsxsrc/components/MenuBar/TimestampQueryBox.css
📚 Learning: in `src/components/popups/popupmessagebox.tsx`, future enhancements may include adding multiple acti...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/components/centralcontainer/sidebar/sidebartabs/searchtabpanel/index.css`, the use of `!impo...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.985Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsxsrc/components/MenuBar/TimestampQueryBox.css
📚 Learning: in `new-log-viewer/src/components/centralcontainer/sidebar/index.tsx`, when using `useeffect` to reg...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/components/MenuBar/index.tsx
🧬 Code Graph Analysis (1)
src/components/MenuBar/TimestampQueryBox.tsx (3)
src/typings/states.ts (1)
UI_ELEMENT(105-105)src/utils/url/index.ts (1)
updateWindowUrlHashParams(384-384)src/utils/url/urlHash.ts (1)
updateViewHashParams(141-141)
🔇 Additional comments (3)
src/components/MenuBar/TimestampQueryBox.css (1)
1-12: LGTM!The CSS styling is clean and well-structured. Good use of flexbox for layout, CSS custom properties for consistent height, and proper box-sizing. The styling integrates well with the existing menu bar design.
src/components/MenuBar/index.tsx (1)
82-84: LGTM!The integration of the TimestampQueryBox follows the existing pattern of using vertical dividers for visual separation. The positioning between the filename display and NavigationBar is appropriate for this new functionality.
src/components/MenuBar/TimestampQueryBox.tsx (1)
15-21: LGTM!The component documentation clearly describes the functionality and the overall approach is sound. The integration with the existing UI state management and URL hash parameter system is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/MenuBar/TimestampQueryBox.tsx (1)
58-62: Replace DOM manipulation with React ref.Using
document.getElementByIdis an anti-pattern in React. UseuseRefto access the input element instead.Apply this diff to use a ref:
+import {useRef} from "react"; + const TimestampQueryBox = () => { const uiState = useUiStore((state) => state.uiState); + const inputRef = useRef<HTMLInputElement>(null); + return ( <Box className={"timestamp-query-box"}> <input + ref={inputRef} className={"timestamp-query-box-input"}Then update the onClick handler:
onClick={() => { - const input = document.getElementById( - "timestamp-query-box-input" - ) as HTMLInputElement; - handleTimestampQuery(input.value); + handleTimestampQuery(inputRef.current?.value || ""); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/MenuBar/TimestampQueryBox.css(1 hunks)src/components/MenuBar/TimestampQueryBox.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryBox.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
📚 Learning: the search button in the `menubar` component (`new-log-viewer/src/components/menubar/index.tsx`) is ...
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/components/popups/popupmessagebox.tsx`, future enhancements may include adding multiple acti...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/components/centralcontainer/sidebar/sidebartabs/searchtabpanel/index.css`, the use of `!impo...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.985Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/TimestampQueryBox.css
📚 Learning: in `src/services/formatters/yscopeformatter/fieldformatters/timestampformatter.ts`, the `converttoda...
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts:24-36
Timestamp: 2024-11-18T01:34:54.885Z
Learning: In `src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts`, the `convertToDayjsTimestamp` function already includes error handling for invalid inputs, so wrapping it in try-catch in the `TimestampFormatter.formatField` method is unnecessary.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: the `updatewindowurlhashparams` function in `src/utils/url.ts` doesn't throw errors, so error handli...
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `new-log-viewer/src/components/centralcontainer/sidebar/index.tsx`, when using `useeffect` to reg...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in this project, avoid using `react.fc` due to concerns highlighted in https://github.com/facebook/c...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: the `paneltabs` component is already wrapped with `react.forwardref` and passes the `ref` to the app...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/SidebarContainer/index.tsx:107-107
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The `PanelTabs` component is already wrapped with `React.forwardRef` and passes the `ref` to the appropriate child element.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (1)
src/components/MenuBar/TimestampQueryBox.css (1)
1-29: LGTM! Well-structured CSS with proper theming support.The CSS implementation follows good practices with proper use of CSS custom properties for theming, standard dark mode support via
prefers-color-scheme, and appropriate flexbox layout for the component structure.
…20px; add collapse icon and center text; disable the timestamp query calendar icon by default
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/components/MenuBar/index.tsx (1)
27-27: Remove file extension from import statement.The import should not include the
.tsxextension as it's not necessary and inconsistent with other imports in the file.Apply this diff:
-import TimestampQueryBox from "./TimestampQueryBox"; +import TimestampQueryBox from "./TimestampQueryBox";src/components/MenuBar/TimestampQueryBox.tsx (2)
20-27: Add validation for invalid timestamp conversion.The
new Date().getTime()call can returnNaNfor invalid datetime strings, which should be validated before updating URL parameters.Apply this diff to add proper validation:
const handleTimestampQuery = (datetime: string) => { if (datetime) { - console.error(datetime); - const timestamp = new Date(`${datetime}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); + try { + const timestamp = new Date(`${datetime}Z`).getTime(); + if (false == isNaN(timestamp)) { + updateWindowUrlHashParams({timestamp: timestamp}); + updateViewHashParams(); + } + } catch (error) { + console.warn("Invalid datetime format:", datetime); + } } };
60-64: Replace DOM manipulation with React ref.Using
document.getElementByIdis an anti-pattern in React. UseuseRefto access the input element instead.Apply this diff to use a ref:
+import {useRef} from "react"; + const TimestampQueryBox = () => { const uiState = useUiStore((state) => state.uiState); + const inputRef = useRef<HTMLInputElement>(null); + return ( <Box className={"timestamp-query-box"}> <input + ref={inputRef} className={"timestamp-query-box-input"} - id={"timestamp-query-box-input"}Then update the onClick handler:
onClick={() => { - const input = document.getElementById( - "timestamp-query-box-input" - ) as HTMLInputElement; - - handleTimestampQuery(input.value); + handleTimestampQuery(inputRef.current?.value || ""); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/MenuBar/TimestampQueryBox.css(1 hunks)src/components/MenuBar/TimestampQueryBox.tsx(1 hunks)src/components/MenuBar/index.css(1 hunks)src/components/MenuBar/index.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
📚 Learning: in `src/components/centralcontainer/sidebar/sidebartabs/searchtabpanel/index.css`, the use of `!impo...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.985Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
Applied to files:
src/components/MenuBar/index.csssrc/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsxsrc/components/MenuBar/TimestampQueryBox.css
📚 Learning: in `new-log-viewer/src/components/centralcontainer/sidebar/index.tsx`, when using `useeffect` to reg...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/components/MenuBar/index.csssrc/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: the search button in the `menubar` component (`new-log-viewer/src/components/menubar/index.tsx`) is ...
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/index.csssrc/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: in `src/components/popups/popupmessagebox.tsx`, future enhancements may include adding multiple acti...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: in `src/services/formatters/yscopeformatter/fieldformatters/timestampformatter.ts`, the `converttoda...
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts:24-36
Timestamp: 2024-11-18T01:34:54.885Z
Learning: In `src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts`, the `convertToDayjsTimestamp` function already includes error handling for invalid inputs, so wrapping it in try-catch in the `TimestampFormatter.formatField` method is unnecessary.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: the `updatewindowurlhashparams` function in `src/utils/url.ts` doesn't throw errors, so error handli...
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in this project, avoid using `react.fc` due to concerns highlighted in https://github.com/facebook/c...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: the `paneltabs` component is already wrapped with `react.forwardref` and passes the `ref` to the app...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/SidebarContainer/index.tsx:107-107
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The `PanelTabs` component is already wrapped with `React.forwardRef` and passes the `ref` to the appropriate child element.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: when suggesting to add validation for form inputs in `settingsdialog.tsx`, the user considered it no...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
📚 Learning: in the `getpanelwidth` function in `sidebar/index.tsx`, it's acceptable not to handle `nan` explicit...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:23-26
Timestamp: 2024-09-28T02:35:29.384Z
Learning: In the `getPanelWidth` function in `Sidebar/index.tsx`, it's acceptable not to handle `NaN` explicitly since it would only occur if `--ylv-panel-width` is not set due to coding errors.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in javascript/typescript module imports, `index` is automatically resolved when importing from a dir...
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/decoders/ClpIrDecoder.ts:0-0
Timestamp: 2024-11-18T01:36:22.048Z
Learning: In JavaScript/TypeScript module imports, `index` is automatically resolved when importing from a directory, so specifying `index` in the import path is unnecessary.
Applied to files:
src/components/MenuBar/index.tsx
📚 Learning: in the yscope-log-viewer codebase, when using zustand stores in react components, the preferred patt...
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
src/components/MenuBar/index.tsx
📚 Learning: when importing from npm packages, use the public api entrypoint (e.g., `import {statecreator} from "...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Applied to files:
src/components/MenuBar/index.tsx
📚 Learning: in the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is ...
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from 'mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Applied to files:
src/components/MenuBar/index.tsx
📚 Learning: in 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
🧬 Code Graph Analysis (1)
src/components/MenuBar/TimestampQueryBox.tsx (3)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(384-384)src/utils/url/urlHash.ts (1)
updateViewHashParams(141-141)src/typings/states.ts (1)
UI_ELEMENT(105-105)
🔇 Additional comments (5)
src/components/MenuBar/index.css (1)
51-63: LGTM! Well-designed expandable UI with smooth transitions.The CSS implementation effectively handles the timestamp query box's expand/collapse functionality using max-width transitions and overflow control.
src/components/MenuBar/TimestampQueryBox.css (1)
1-30: LGTM! Proper theme support and responsive design.The CSS effectively implements light/dark theme support using custom properties and media queries, with appropriate styling for the timestamp input component.
src/components/MenuBar/index.tsx (2)
40-51: LGTM! Proper state management and callback implementation.The useState hook and useCallback for toggle functionality are implemented correctly, following React best practices.
93-110: LGTM! Clean conditional rendering and UI integration.The integration of the timestamp query UI with conditional icon switching and CSS class application is well-implemented and maintains clean separation of concerns.
src/components/MenuBar/TimestampQueryBox.tsx (1)
37-71: Component concept and structure are well-designed.The overall approach of combining a datetime input with a search button and proper event handling (Enter key and click) provides good user experience, despite the implementation issues noted above.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/MenuBar/TimestampQueryBox.tsx (2)
20-26: Add validation for invalid timestamp conversion.The
new Date().getTime()call can returnNaNfor invalid datetime strings, which should be validated before updating URL parameters.const handleTimestampQuery = (datetime: string) => { if (datetime) { - const timestamp = new Date(`${datetime}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); + try { + const timestamp = new Date(`${datetime}Z`).getTime(); + if (false == isNaN(timestamp)) { + updateWindowUrlHashParams({timestamp: timestamp}); + updateViewHashParams(); + } + } catch (error) { + console.warn("Invalid datetime format:", datetime); + } } };
59-63: Replace DOM manipulation with React ref for better React practices.Using
document.getElementByIdis an anti-pattern in React. Consider usinguseReffor direct element access, though the current approach is acceptable since the element has a unique ID.+import {useRef} from "react"; + const TimestampQueryBox = () => { const uiState = useUiStore((state) => state.uiState); + const inputRef = useRef<HTMLInputElement>(null); + return ( <Box className={"timestamp-query-box"}> <input + ref={inputRef} className={"timestamp-query-box-input"} - id={"timestamp-query-box-input"} // ... other props onClick={() => { - const input = document.getElementById( - "timestamp-query-box-input" - ) as HTMLInputElement; - handleTimestampQuery(input.value); + handleTimestampQuery(inputRef.current?.value || ""); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/MenuBar/TimestampQueryBox.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryBox.tsx
🧠 Learnings (11)
📓 Common learnings
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
📚 Learning: the search button in the `menubar` component (`new-log-viewer/src/components/menubar/index.tsx`) is ...
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/components/popups/popupmessagebox.tsx`, future enhancements may include adding multiple acti...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/components/centralcontainer/sidebar/sidebartabs/searchtabpanel/index.css`, the use of `!impo...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.985Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `src/services/formatters/yscopeformatter/fieldformatters/timestampformatter.ts`, the `converttoda...
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts:24-36
Timestamp: 2024-11-18T01:34:54.885Z
Learning: In `src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts`, the `convertToDayjsTimestamp` function already includes error handling for invalid inputs, so wrapping it in try-catch in the `TimestampFormatter.formatField` method is unnecessary.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: the `updatewindowurlhashparams` function in `src/utils/url.ts` doesn't throw errors, so error handli...
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in `new-log-viewer/src/components/centralcontainer/sidebar/index.tsx`, when using `useeffect` to reg...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in this project, avoid using `react.fc` due to concerns highlighted in https://github.com/facebook/c...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: the `paneltabs` component is already wrapped with `react.forwardref` and passes the `ref` to the app...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/SidebarContainer/index.tsx:107-107
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The `PanelTabs` component is already wrapped with `React.forwardRef` and passes the `ref` to the appropriate child element.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: when suggesting to add validation for form inputs in `settingsdialog.tsx`, the user considered it no...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: in the `getpanelwidth` function in `sidebar/index.tsx`, it's acceptable not to handle `nan` explicit...
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:23-26
Timestamp: 2024-09-28T02:35:29.384Z
Learning: In the `getPanelWidth` function in `Sidebar/index.tsx`, it's acceptable not to handle `NaN` explicitly since it would only occur if `--ylv-panel-width` is not set due to coding errors.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
🧬 Code Graph Analysis (1)
src/components/MenuBar/TimestampQueryBox.tsx (3)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(384-384)src/utils/url/urlHash.ts (1)
updateViewHashParams(141-141)src/typings/states.ts (1)
UI_ELEMENT(105-105)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/MenuBar/index.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/index.tsx
🧠 Learnings (1)
📚 Learning: 2024-10-09T01:22:57.841Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/index.tsx
🔇 Additional comments (4)
src/components/MenuBar/index.tsx (4)
29-29: Import path consistency looks goodThanks for importing the component without the .tsx extension; that aligns with project conventions.
42-42: Local UI state for timestamp query visibility is appropriateThe dedicated boolean state keeps the MenuBar logic simple and avoids coupling with stores. Looks good.
129-134: Good adherence to boolean style guidelineUsing false === isDisabled(...) avoids negation and matches the repository’s preference.
55-57: Follow repo guideline: avoid the ! operatorPer coding guidelines for TS/TSX, prefer false == over using the ! operator.
- setShowTimestampQuery((prev) => !prev); + setShowTimestampQuery((prev) => false == prev);⛔ Skipped due to learnings
Learnt from: Henry8192 PR: y-scope/yscope-log-viewer#196 File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58 Timestamp: 2025-03-03T18:10:02.451Z Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.Learnt from: Henry8192 PR: y-scope/yscope-log-viewer#152 File: src/components/AppController.tsx:42-42 Timestamp: 2025-07-15T16:29:02.609Z Learning: In the y-scope/yscope-log-viewer repository, the team never uses `==` for comparisons, preferring strict equality (`===`) instead. This overrides the coding guidelines that suggest using `false == <expression>` rather than `!<expression>`.
Henry8192
changed the title
TimestampQueryBox for simpler timestamp queries.
junhaoliao
requested changes
Aug 26, 2025
| const {mode, systemMode} = useColorScheme(); | ||
| return ( | ||
| <Box className={"timestamp-query-box"}> | ||
| <input |
There was a problem hiding this comment.
can we change the tooltip of the calendar button from "Show local date and time picker" -> "Show UTC picker" / "Show picker" so it's less misleading?
There was a problem hiding this comment.
I think this is controlled by browser.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
src/components/MenuBar/TimestampQueryBox.tsx (3)
54-55: Extract helper for default UTC valueKeep UTC semantics, but extract as a named helper for clarity (echoing prior feedback).
- defaultValue={new Date().toISOString() - .slice(0, -1)} + defaultValue={getDatetimeUtcNow()}Add above the component or within it:
const getDatetimeUtcNow = (): string => new Date().toISOString().slice(0, -1);
46-53: Use a React ref instead of DOM lookup; remove idDirect DOM access is an anti-pattern in React. Use
useRefand drop theid. Also simplifies the click handler.+import {useRef} from "react"; ... const TimestampQueryBox = () => { const uiState = useUiStore((state) => state.uiState); const {mode, systemMode} = useColorScheme(); + const inputRef = useRef<HTMLInputElement>(null); ... - <input + <input disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} - id={"timestamp-query-box-input"} + ref={inputRef} step={"0.1"} title={"Timestamp to seek to in UTC"} type={"datetime-local"} className={`timestamp-query-box-input ${(("system" === mode) ? systemMode : mode) ?? THEME_NAME.DARK}`} ... - onClick={() => { - const input = document.getElementById( - "timestamp-query-box-input", - ) as HTMLInputElement; - - handleTimestampQuery(input.value); - }} + onClick={() => { + handleTimestampQuery(inputRef.current?.value ?? ""); + }}Also applies to: 66-71
49-49: Clarify copy to reflect behaviourMake tooltip/title explicit: “Jump to the first log event at or after this UTC time”.
- title={"Timestamp to seek to in UTC"} + title={"Jump to the first log event at or after this UTC time (UTC)"} ... - tooltipTitle={"Seek to timestamp"} + tooltipTitle={"Jump to the first log event at or after this UTC time"}Also applies to: 63-65
src/components/MenuBar/index.tsx (3)
16-16: Collapse icon direction for LTRUse a left-pointing variant for “collapse” in LTR; consider switching based on layout direction if RTL is supported.
-import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowRight"; +import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowLeft";
102-102: Tooltip placement consistencyUnless there’s a specific reason, align with common usage and use
"bottom".- tooltipPlacement={"bottom-start"} + tooltipPlacement={"bottom"}
100-111: Scope disabled gating and add basic a11y attributesOptionally gate the toggle with a dedicated UI element (e.g.,
TIMESTAMP_QUERY_TOGGLE) instead ofNAVIGATION_BAR, and expose state to AT with ARIA props. Keep changes local here; broader store wiring can be a follow-up.<MenuBarIconButton - disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} + disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} tooltipPlacement={"bottom-start"} tooltipTitle={showTimestampQuery ? "Collapse" : "Seek to timestamp"} - onClick={toggleTimestampQuery} + onClick={toggleTimestampQuery} + aria-label={showTimestampQuery ? "Collapse timestamp query" : "Open timestamp query"} + aria-expanded={showTimestampQuery} + aria-controls={"timestamp-query-region"} + aria-pressed={showTimestampQuery} > ... - <div + <div className={`timestamp-query-wrapper ${showTimestampQuery ? "expanded" : ""}`} + id={"timestamp-query-region"} + role={"region"} + aria-label={"Timestamp query panel"} + aria-hidden={false == showTimestampQuery} >I can open a follow-up to introduce
UI_ELEMENT.TIMESTAMP_QUERY_TOGGLEif desired.Also applies to: 113-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/MenuBar/TimestampQueryBox.css(1 hunks)src/components/MenuBar/TimestampQueryBox.tsx(1 hunks)src/components/MenuBar/index.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryBox.tsxsrc/components/MenuBar/index.tsx
🧠 Learnings (10)
📚 Learning: 2024-11-14T04:35:00.985Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.985Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-10-08T15:52:50.753Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.css:6-8
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this codebase, using `!important` is acceptable when overriding styles from JoyUI components.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-10-08T15:52:50.753Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/PanelTabs/TooltipTab.css:0-0
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When overriding JoyUI styles, `!important` may be necessary to ensure custom styles take precedence.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-10-08T15:52:50.753Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/LogLevelChip.css:6-6
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When overriding JoyUI's default styling, it's acceptable to use `!important` in CSS in this project.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-09-28T01:00:02.202Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/PanelTabs/TooltipTab.css:0-0
Timestamp: 2024-09-28T01:00:02.202Z
Learning: Using `!important` is acceptable when overriding JoyUI styles in this project.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-09-28T01:11:13.412Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/ResizeHandle.css:0-0
Timestamp: 2024-09-28T01:11:13.412Z
Learning: CSS-in-JS is avoided due to performance concerns until the Pigment engine is supported in JoyUI.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-09-28T01:11:13.412Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/ResizeHandle.css:0-0
Timestamp: 2024-09-28T01:11:13.412Z
Learning: The CSS property names are from JoyUI and cannot be easily overridden.
Applied to files:
src/components/MenuBar/TimestampQueryBox.css
📚 Learning: 2024-10-09T01:22:57.841Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: 2024-11-18T01:34:54.885Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts:24-36
Timestamp: 2024-11-18T01:34:54.885Z
Learning: In `src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts`, the `convertToDayjsTimestamp` function already includes error handling for invalid inputs, so wrapping it in try-catch in the `TimestampFormatter.formatField` method is unnecessary.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryBox.tsx
🧬 Code graph analysis (2)
src/components/MenuBar/TimestampQueryBox.tsx (4)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)src/utils/url/urlHash.ts (1)
updateViewHashParams(138-138)src/typings/states.ts (1)
UI_ELEMENT(105-105)src/typings/config.ts (1)
THEME_NAME(47-47)
src/components/MenuBar/index.tsx (1)
src/typings/states.ts (1)
UI_ELEMENT(105-105)
junhaoliao
requested changes
Sep 2, 2025
Comment on lines
80
to
86
| <MenuBarIconButton | ||
| disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} | ||
| tooltipTitle={"Search by timestamp"} | ||
| onClick={searchByTimestamp} | ||
| > | ||
| <SearchIcon/> | ||
| </MenuBarIconButton> |
There was a problem hiding this comment.
we should avoid showing dividers between the datetime input and the hide / search buttons; otherwise they look like separate components especially when they are placed next to the page navigation component and the export button. i.e.,
i think this is better ->
There was a problem hiding this comment.
to achieve that, we could set the startDecorator and endDecorator on the Input component. for changing the collapse state, we can pass a callback like onTimestampQueryInputCollapse to TimestampQueryBox
There was a problem hiding this comment.
Since the collapse button and calendar button (search by timestamp) are outside of Input, what are your thoughts to refactor that in start decorator?
I mean, when the Input is collapsed, the collapse button will also be "collapsed", right?
One hacky way I can think of is to make the calendar button disappear on click, and display the collapse button as the start decorator of Input. What do you think?
There was a problem hiding this comment.
to make the calendar button disappear on click, and display the collapse button as the start decorator of Input.
yup that's what i had in mind
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (3)
src/components/MenuBar/index.css (1)
51-55: Smoother expand/collapse animation with a perf hintAdd will-change to help the browser plan the transition; also respect reduced motion.
.timestamp-query-input-wrapper { overflow: hidden; max-width: 0; transition: max-width 0.3s ease; + will-change: max-width; } + +@media (prefers-reduced-motion: reduce) { + .timestamp-query-input-wrapper { + transition: none; + } +}src/components/MenuBar/index.tsx (1)
24-24: Remove file extension from importKeep imports consistent with the rest of the codebase.
-import TimestampQueryInputContainer from "./TimestampQueryInputContainer.tsx"; +import TimestampQueryInputContainer from "./TimestampQueryInputContainer";src/stores/viewStore/createViewEventSlice.ts (1)
12-13: Compute default timestamp at slice creation (not at import time).Module-level new Date() is stale if the store loads long before first render. Initialise at slice creation.
Apply this diff to avoid computing “now” at import:
- dateTimeString: new Date().toISOString() - .slice(0, -1), + dateTimeString: "",Then set a fresh default inside the slice (outside selected range):
const createViewEventSlice: StateCreator<ViewState, [], [], ViewEventSlice> = (set) => { const nowUtcNoZ = new Date().toISOString().slice(0, -1); return { ...VIEW_EVENT_DEFAULT, dateTimeString: nowUtcNoZ, setLogEventNum: (logEventNum: number) => set({ logEventNum }), setDateTimeString: (dateTimeString: string) => set({ dateTimeString }), }; };Verify the input accepts fractional seconds; if not, trim “.sss” in the default:
#!/bin/bash # Check whether the timestamp input uses datetime-local and supports step for seconds/fractions rg -nP '\btype\s*=\s*["\']datetime-local["\']' src/components/MenuBar/TimestampQueryInput.tsx -C2 rg -nP '\bstep\s*=' src/components/MenuBar/TimestampQueryInput.tsx -C2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/MenuBar/TimestampQueryInput.css(1 hunks)src/components/MenuBar/TimestampQueryInput.tsx(1 hunks)src/components/MenuBar/TimestampQueryInputContainer.css(1 hunks)src/components/MenuBar/TimestampQueryInputContainer.tsx(1 hunks)src/components/MenuBar/index.css(1 hunks)src/components/MenuBar/index.tsx(3 hunks)src/stores/viewStore/createViewEventSlice.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryInput.tsxsrc/components/MenuBar/TimestampQueryInputContainer.tsxsrc/stores/viewStore/createViewEventSlice.tssrc/components/MenuBar/index.tsx
🔇 Additional comments (1)
src/stores/viewStore/createViewEventSlice.ts (1)
29-31: Setter LGTM.
Straightforward Zustand partial update.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/components/MenuBar/index.tsx (1)
88-92: Gate the TimestampQueryContainer (and its dividers) behind file loadAvoid showing inactive UI when no file is loaded; wrap with a simple check, keeping dividers inside the same conditional.
- <Divider orientation={"vertical"}/> - <TimestampQueryContainer/> - - <Divider orientation={"vertical"}/> + {0 < fileName.length && ( + <> + <Divider orientation={"vertical"}/> + <TimestampQueryContainer/> + <Divider orientation={"vertical"}/> + </> + )}src/components/MenuBar/TimestampQueryContainer.tsx (2)
9-9: Collapse icon direction for LTRUse the left‑pointing variant to better communicate “collapse” in LTR layouts.
-import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowRight"; +import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowLeft";
36-36: Refine disabled gating to a dedicated UI elementUsing NAVIGATION_BAR is broad; introduce UI_ELEMENT.TIMESTAMP_QUERY_TOGGLE for independent control. Thread it through the UI store and replace the check here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/MenuBar/TimestampQueryContainer.css(1 hunks)src/components/MenuBar/TimestampQueryContainer.tsx(1 hunks)src/components/MenuBar/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryContainer.tsxsrc/components/MenuBar/index.tsx
🧬 Code graph analysis (1)
src/components/MenuBar/TimestampQueryContainer.tsx (1)
src/typings/states.ts (1)
UI_ELEMENT(105-105)
🔇 Additional comments (2)
src/components/MenuBar/index.tsx (1)
88-91: Verify no double vertical rules appearWith multiple adjacent sections and conditionals, please visually confirm there are no back‑to‑back vertical Dividers around the filename/timestamp areas.
src/components/MenuBar/TimestampQueryContainer.tsx (1)
29-31: Follow guideline: avoid!prev; usefalse == prevConform to the repo’s boolean style.
- const toggleTimestampQuery = useCallback(() => { - setShowTimestampQuery((prev) => !prev); - }, []); + const toggleTimestampQuery = useCallback(() => { + setShowTimestampQuery((prev) => false == prev); + }, []);⛔ Skipped due to learnings
Learnt from: Henry8192 PR: y-scope/yscope-log-viewer#196 File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58 Timestamp: 2025-03-03T18:10:02.451Z Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
…nction to handleDateTimeInputChange
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/components/MenuBar/index.tsx (1)
89-90: Optionally gate render on file load to reduce idle UI noiseHide the timestamp UI until a file is loaded (or when the UI element is disabled).
- <TimestampQueryContainer/> + {(0 < fileName.length) && <TimestampQueryContainer/>}src/components/MenuBar/TimestampQueryInput.tsx (4)
39-44: Handle IME composition and use type-only event importsPrevents premature submit during composition and removes reliance on the React namespace types.
-import React, {useCallback} from "react"; +import {useCallback, type ChangeEvent, type KeyboardEvent} from "react";- const handleKeyboardEnterPress = useCallback((e: React.KeyboardEvent<HTMLInputElement>) => { - if ("Enter" === e.key) { + const handleKeyboardEnterPress = useCallback((e: KeyboardEvent<HTMLInputElement>) => { + // @ts-expect-error Joy/React event exposes nativeEvent; guard IME composition + if ((false == (e as any)?.nativeEvent?.isComposing) && "Enter" === e.key) { handleTimestampQuery(); } }, [handleTimestampQuery]); - const handleDateTimeInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { + const handleDateTimeInputChange = useCallback((e: ChangeEvent<HTMLInputElement>) => { const {setDateTimeString} = useViewStore.getState(); - setDateTimeString(e.currentTarget.value); + setDateTimeString(e.currentTarget.value.trim()); }, []);Also applies to: 45-48
52-70: A11y and UX: expose label, allow seconds/ms, and label the icon buttonMake the control self-describing and allow precise time entry; align button with other MenuBar buttons.
<Tooltip title={"Jump to the nearest log event at/after this UTC time"}> <Input disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} size={"sm"} title={"Timestamp to seek to in UTC"} type={"datetime-local"} value={dateTimeString} + placeholder={"YYYY-MM-DDThh:mm[:ss[.SSS]]"} + step={"0.001"} + aria-label={"UTC timestamp"} endDecorator={ <MenuBarIconButton disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} tooltipTitle={"Search by timestamp"} + tooltipPlacement={"bottom-start"} + aria-label={"Search by timestamp"} onClick={handleTimestampQuery} > <SearchIcon/> </MenuBarIconButton> } onChange={handleDateTimeInputChange} onKeyDown={handleKeyboardEnterPress}/> </Tooltip>
52-70: Clarify UTC semantics for a datetime-local controlAdd a visible “UTC” badge to avoid misinterpretation.
- <Input + <Input + startDecorator={<span aria-hidden={"true"}>UTC</span>}
33-37: Guard against invalid/empty input to avoidtimestamp=NaNand broken navigationValidate/normalise the value, treat as UTC, and no-op on invalid input.
- const handleTimestampQuery = useCallback(() => { - const timestamp = new Date(`${dateTimeString}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); - }, [dateTimeString]); + const handleTimestampQuery = useCallback(() => { + const raw = dateTimeString?.trim() ?? ""; + const hasValue = 0 < raw.length; + if (false == hasValue) { + return; + } + // Ensure UTC interpretation. + const iso = raw.endsWith("Z") ? raw : `${raw}Z`; + const ts = Date.parse(iso); + if (false == Number.isFinite(ts)) { + return; + } + updateWindowUrlHashParams({timestamp: Math.trunc(ts)}); + updateViewHashParams(); + }, [dateTimeString]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/MenuBar/TimestampQueryInput.tsx(1 hunks)src/components/MenuBar/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/index.tsxsrc/components/MenuBar/TimestampQueryInput.tsx
🧠 Learnings (1)
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
🧬 Code graph analysis (1)
src/components/MenuBar/TimestampQueryInput.tsx (3)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)src/utils/url/urlHash.ts (1)
updateViewHashParams(154-154)src/typings/states.ts (1)
UI_ELEMENT(105-105)
🔇 Additional comments (1)
src/components/MenuBar/index.tsx (1)
24-24: LGTM: import pathImporting TimestampQueryContainer without the extension is correct and consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/stores/viewStore/types.ts (1)
25-33: Document UTC format and tighten the type for dateTimeString.Add JSDoc to make the UTC/no-
Zcontract explicit and mirror it on the setter. Optionally, brand the type to prevent accidental local-time strings.interface ViewEventValues { logEventNum: number; - dateTimeString: string; + /** + * ISO 8601 UTC string without trailing 'Z', e.g. "2025-09-02T12:34:56.789". + * Intended for use with <input type="datetime-local"> while representing UTC. + */ + dateTimeString: string; } interface ViewEventActions { setLogEventNum: (newLogEventNum: number) => void; - setDateTimeString: (newDateTimeString: string) => void; + /** + * Expects an ISO 8601 UTC string without 'Z' (see dateTimeString contract). + */ + setDateTimeString: (newDateTimeString: string) => void; }Optional branded type to reduce misuse (apply where appropriate):
export type UtcDateTimeString = string & { __brand: "UtcDateTimeString" };src/components/MenuBar/index.tsx (1)
91-93: Optionally gate rendering to reduce idle UI noise.Hide TimestampQueryContainer when the NAVIGATION_BAR is disabled (or until a file is loaded).
- <TimestampQueryContainer/> + {(false === isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)) && ( + <TimestampQueryContainer/> + )}src/utils/url/urlHash.ts (1)
20-31: Guard against invalid timestamps before toISOString().new Date(NaN|±Infinity).toISOString() throws RangeError. Add a finiteness check and a no-op if invalid. Also clarify the units in the doc.
-/** - * Converts a timestamp to an ISO 8601 date-time string (without the 'Z' suffix) - * - * @param timestamp - */ +/** + * Converts an epoch-millis timestamp to an ISO 8601 UTC date-time string + * (without the trailing 'Z') and updates the view state's dateTimeString. + * + * @param timestamp Epoch milliseconds. + */ const updateDateTimeString = (timestamp: number) => { - const dateTimeString = new Date(timestamp).toISOString() - .slice(0, -1); + if (false == Number.isFinite(timestamp)) { + return; + } + const iso = new Date(timestamp).toISOString(); + const dateTimeString = iso.slice(0, -1); const {setDateTimeString} = useViewStore.getState(); - setDateTimeString(dateTimeString); + setDateTimeString(dateTimeString); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/MenuBar/index.tsx(2 hunks)src/stores/viewStore/types.ts(1 hunks)src/utils/url/urlHash.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/viewStore/types.tssrc/components/MenuBar/index.tsxsrc/utils/url/urlHash.ts
🧠 Learnings (3)
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-11-18T01:34:54.885Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts:24-36
Timestamp: 2024-11-18T01:34:54.885Z
Learning: In `src/services/formatters/YScopeFormatter/FieldFormatters/TimestampFormatter.ts`, the `convertToDayjsTimestamp` function already includes error handling for invalid inputs, so wrapping it in try-catch in the `TimestampFormatter.formatField` method is unnecessary.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-07-24T22:01:29.322Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#347
File: src/utils/math.ts:44-45
Timestamp: 2025-07-24T22:01:29.322Z
Learning: In TypeScript with strong typing, function parameters typed as `number` cannot be undefined at runtime when called correctly, so checking if such parameters are undefined is unnecessary.
Applied to files:
src/utils/url/urlHash.ts
🔇 Additional comments (1)
src/components/MenuBar/index.tsx (1)
24-24: LGTM: correct import without extension.
Comment on lines
72
to
76
| if (timestamp !== URL_HASH_PARAMS_DEFAULT.timestamp) { | ||
| updateDateTimeString(timestamp); | ||
|
|
||
| return { | ||
| code: CURSOR_CODE.TIMESTAMP, |
There was a problem hiding this comment.
🧹 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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/components/MenuBar/TimestampQueryInput.tsx (3)
46-58: Guard against IME composition and trim input on changePrevent premature submit during IME composition; normalise input by trimming.
- const handleKeyboardEnterPress = useCallback( - (e: React.KeyboardEvent<HTMLInputElement>) => { - if ("Enter" === e.key) { - handleTimestampQuery(); - } - }, - [handleTimestampQuery], - ); + const handleKeyboardEnterPress = useCallback( + (e: KeyboardEvent<HTMLInputElement>) => { + // Avoid triggering while composing (IME) + // @ts-expect-error nativeEvent is present at runtime on React events + if ((false == (e as any)?.nativeEvent?.isComposing) && "Enter" === e.key) { + handleTimestampQuery(); + } + }, + [handleTimestampQuery], + ); - const handleDateTimeInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { + const handleDateTimeInputChange = useCallback((e: ChangeEvent<HTMLInputElement>) => { const {setDateTimeString} = useViewStore.getState(); - setDateTimeString(e.currentTarget.value); + setDateTimeString(e.currentTarget.value.trim()); }, []);
40-44: Validate/normalise timestamp and avoid overwriting it via hash syncUnvalidated input can yield NaN, resulting in
timestamp=NaN. Also, callingupdateViewHashParams()after settingtimestampcan overwrite it. Validate, append Z, and set the timestamp last.- const handleTimestampQuery = useCallback(() => { - const timestamp = new Date(`${dateTimeString}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); - }, [dateTimeString]); + const handleTimestampQuery = useCallback(() => { + const raw = dateTimeString?.trim(); + if (!raw) return; + const iso = raw.endsWith("Z") ? raw : `${raw}Z`; + const ts = Date.parse(iso); + if (false == Number.isFinite(ts)) return; + // Write view params first, then set timestamp last to avoid being reset. + updateViewHashParams(); + updateWindowUrlHashParams({timestamp: Math.floor(ts)}); + }, [dateTimeString]);Run to verify whether
updateViewHashParamsoverwritestimestamp:#!/bin/bash # Inspect hash writers and defaulting behaviour for `timestamp` rg -n -C3 '\bupdateViewHashParams\b' src rg -n -C3 '\btimestamp\b' src/utils/url src | sed -n '1,200p'
66-91: Improve input/button a11y and clarity for UTC semanticsAdd placeholder, step for ms, and accessible names; clarify UTC to avoid confusion with datetime-local controls.
<Tooltip title={"Jump to the nearest log event at/after this UTC time"}> <Input disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} size={"sm"} title={"Timestamp to seek to in UTC"} type={"datetime-local"} value={dateTimeString} + placeholder={"YYYY-MM-DDThh:mm[:ss[.SSS]] (UTC)"} + step={"0.001"} + aria-label={"UTC timestamp"} endDecorator={ <MenuBarIconButton disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} tooltipTitle={"Search by timestamp"} + tooltipPlacement={"bottom-start"} + aria-label={"Search by timestamp"} onClick={handleTimestampQuery} > <SearchIcon/> </MenuBarIconButton> } startDecorator={ <MenuBarIconButton + aria-label={"Collapse timestamp query"} onClick={collapseTimestampQueryInput} > <CollapseIcon/> </MenuBarIconButton> } onChange={handleDateTimeInputChange} onKeyDown={handleKeyboardEnterPress}/> </Tooltip>Optional: render a small static “UTC” badge/label adjacent to the input for added clarity.
src/components/MenuBar/TimestampQueryContainer.tsx (1)
35-41: Add minimal a11y state to toggle and controlled regionExpose expanded state and control relationship; mark region appropriately. Scoped, low-impact.
<MenuBarIconButton disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} tooltipTitle={"Search by timestamp"} + aria-label={showTimestampQuery ? "Collapse timestamp query" : "Open timestamp query"} + aria-expanded={showTimestampQuery} + aria-controls={"timestamp-query-region"} + aria-pressed={showTimestampQuery} onClick={toggleTimestampQuery} > <CalendarTodayIcon/> </MenuBarIconButton> )} <div className={`timestamp-query-input-wrapper ${showTimestampQuery ? "expanded" : ""}`} + id={"timestamp-query-region"} + role={"region"} + aria-label={"Timestamp query panel"} + aria-hidden={false == showTimestampQuery} > <TimestampQueryInput setShowTimestampQuery={setShowTimestampQuery}/> </div>Also applies to: 43-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/MenuBar/TimestampQueryContainer.tsx(1 hunks)src/components/MenuBar/TimestampQueryInput.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryContainer.tsxsrc/components/MenuBar/TimestampQueryInput.tsx
🧠 Learnings (2)
📚 Learning: 2024-09-28T02:32:08.882Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-09-28T02:32:08.882Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Applied to files:
src/components/MenuBar/TimestampQueryContainer.tsx
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
🧬 Code graph analysis (2)
src/components/MenuBar/TimestampQueryContainer.tsx (1)
src/typings/states.ts (1)
UI_ELEMENT(105-105)
src/components/MenuBar/TimestampQueryInput.tsx (3)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(389-389)src/utils/url/urlHash.ts (1)
updateViewHashParams(174-174)src/typings/states.ts (1)
UI_ELEMENT(105-105)
🔇 Additional comments (1)
src/components/MenuBar/TimestampQueryContainer.tsx (1)
28-30: Follow project style: avoid!in boolean expressionsPrefer
false == <expr>over!<expr>per coding guidelines.- const toggleTimestampQuery = useCallback(() => { - setShowTimestampQuery((prev) => !prev); - }, []); + const toggleTimestampQuery = useCallback(() => { + setShowTimestampQuery((prev) => (false == prev)); + }, []);⛔ Skipped due to learnings
Learnt from: Henry8192 PR: y-scope/yscope-log-viewer#196 File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58 Timestamp: 2025-03-03T18:10:02.451Z Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
…imestampQueryInput` is expanded; however, startDecorator & endDecorator icons are overflowed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
src/components/MenuBar/index.tsx (1)
90-91: Optionally hide timestamp UI when no file is loaded.
Reduces visual noise; render the container only when a file is present or corresponding UI element is enabled.src/components/MenuBar/TimestampQueryInput.tsx (4)
1-1: Use type-only React event imports; drop default React import.
Saves a runtime import and clarifies intent.-import React, {useCallback} from "react"; +import {useCallback, type ChangeEvent, type KeyboardEvent} from "react";
64-91: Improve input UX/accessibility and propagate disabled to buttons.
Clarify UTC, allow ms, add accessible names, and disable decorators with the input.<Tooltip title={"Jump to the nearest log event at/after this UTC time"}> <Input disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} size={"sm"} sx={{height: "100%"}} - title={"Timestamp to seek to in UTC"} + title={"Timestamp to seek to in UTC"} type={"datetime-local"} value={dateTimeString} + placeholder={"YYYY-MM-DDThh:mm[:ss[.SSS]] (UTC)"} + step={"0.001"} + aria-label={"UTC timestamp"} endDecorator={ <MenuBarIconButton - onClick={handleTimestampQuery} + disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} + tooltipTitle={"Search by timestamp"} + tooltipPlacement={"bottom-start"} + aria-label={"Search by timestamp"} + onClick={handleTimestampQuery} > <SearchIcon/> </MenuBarIconButton> } startDecorator={ <MenuBarIconButton - onClick={collapseTimestampQueryInput} + disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} + tooltipTitle={"Collapse"} + tooltipPlacement={"bottom-start"} + aria-label={"Collapse timestamp query"} + onClick={collapseTimestampQueryInput} > <CollapseIcon/> </MenuBarIconButton> } onChange={handleDateTimeInputChange} onKeyDown={handleKeyboardEnterPress}/> </Tooltip>
40-44: Fix NaN/invalid timestamp and preserve hash ordering.
Validate/normalise input, avoid double “Z”, and call view-hash writer before setting timestamp so it isn’t reset.- const handleTimestampQuery = useCallback(() => { - const timestamp = new Date(`${dateTimeString}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); - }, [dateTimeString]); + const handleTimestampQuery = useCallback(() => { + const raw = dateTimeString?.trim(); + if (!raw) return; + const iso = raw.endsWith("Z") ? raw : `${raw}Z`; + const ts = Date.parse(iso); + if (false == Number.isFinite(ts)) return; + // Ensure view params are written first; then set the timestamp. + updateViewHashParams(); + updateWindowUrlHashParams({timestamp: Math.floor(ts)}); + }, [dateTimeString]);
46-53: Guard against IME composition; use type-only event types.
Prevents accidental submit while composing.- const handleKeyboardEnterPress = useCallback( - (e: React.KeyboardEvent<HTMLInputElement>) => { - if ("Enter" === e.key) { + const handleKeyboardEnterPress = useCallback( + (e: KeyboardEvent<HTMLInputElement>) => { + // @ts-expect-error Joy exposes nativeEvent; guard IME composition + if ((false == (e as any)?.nativeEvent?.isComposing) && "Enter" === e.key) { handleTimestampQuery(); } }, [handleTimestampQuery], );src/components/MenuBar/TimestampQueryContainer.tsx (3)
31-33: Toggler implementation is fine as-is.
Keeping!prevaligns with team preference from prior learnings, despite the repo guideline.
41-47: Add tooltip placement and basic a11y to the toggle (optional).
Improves consistency and exposes state to AT; fine to defer to a follow-up.<MenuBarIconButton disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} - tooltipTitle={"Search by timestamp"} + tooltipTitle={"Search by timestamp"} + tooltipPlacement={"bottom-start"} + aria-label={showTimestampQuery ? "Collapse timestamp query" : "Open timestamp query"} + aria-expanded={showTimestampQuery} + aria-controls={"timestamp-query-region"} + aria-pressed={showTimestampQuery} onClick={toggleTimestampQuery} >
51-56: Prevent focus on hidden region (optional).
Mark the collapsed region as hidden for AT; or conditionally unmount if animation isn’t required.<div - className={`timestamp-query-input-wrapper ${showTimestampQuery ? + className={`timestamp-query-input-wrapper ${showTimestampQuery ? "expanded" : ""}`} + id={"timestamp-query-region"} + role={"region"} + aria-label={"Timestamp query panel"} + aria-hidden={false == showTimestampQuery} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/MenuBar/TimestampQueryContainer.tsx(1 hunks)src/components/MenuBar/TimestampQueryInput.tsx(1 hunks)src/components/MenuBar/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryContainer.tsxsrc/components/MenuBar/TimestampQueryInput.tsxsrc/components/MenuBar/index.tsx
🧠 Learnings (4)
📚 Learning: 2024-09-28T02:32:08.882Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-09-28T02:32:08.882Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Applied to files:
src/components/MenuBar/TimestampQueryContainer.tsx
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
📚 Learning: 2024-10-08T15:52:50.753Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
📚 Learning: 2025-03-03T18:10:02.451Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#196
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58
Timestamp: 2025-03-03T18:10:02.451Z
Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
Applied to files:
src/components/MenuBar/index.tsx
🧬 Code graph analysis (1)
src/components/MenuBar/TimestampQueryInput.tsx (3)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(389-389)src/utils/url/urlHash.ts (1)
updateViewHashParams(174-174)src/typings/states.ts (1)
UI_ELEMENT(105-105)
🔇 Additional comments (1)
src/components/MenuBar/index.tsx (1)
24-24: Integration of TimestampQueryContainer looks correct.
Import and placement between filename and NavigationBar are appropriate.Also applies to: 90-91
junhaoliao
requested changes
Sep 12, 2025
There was a problem hiding this comment.
i proposed some changes to:
- improve the abstraction of the
TimestampQueryInputcomponent by accepting aonInputCollapsecallback instead of the React state setter. - rename the input visibility state variable
- nit pick - more concise docs and consistent formatting
for the PR title, how about:
feat(MenuBar): Add timestamp query input for jumping to log events by UTC timestamp (resolves #118).
| * @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 |
There was a problem hiding this comment.
@coderabbitai create a refactoring issue request for this
There was a problem hiding this comment.
🧩 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-statementsrule violation - Suggested refactoring approach to break down the function
- Backlinks to this PR and comment for reference
You can view the issue at: #389
🐰
✅
( ^_^)
📋
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Henry8192
changed the title
TimestampQueryBox for simpler timestamp queries.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
src/components/MenuBar/TimestampQueryInput.tsx (6)
1-1: Use type-only event imports; drop default React importAvoids unnecessary runtime import and aligns with project style.
-import React, {useCallback} from "react"; +import {useCallback, type ChangeEvent, type KeyboardEvent} from "react";And update event annotations below to use KeyboardEvent/ChangeEvent.
9-9: Collapse icon points the wrong way for LTR “collapse”Use a left-pointing variant for collapse in LTR.
-import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowRight"; +import CollapseIcon from "@mui/icons-material/KeyboardDoubleArrowLeft";
39-43: Normalise/validate timestamp and prevent hash reset by call orderingPrevent NaN/invalid inputs and ensure updateViewHashParams doesn’t overwrite the new timestamp by running it first.
- const handleTimestampQuery = useCallback(() => { - const timestamp = new Date(`${dateTimeString}Z`).getTime(); - updateWindowUrlHashParams({timestamp: timestamp}); - updateViewHashParams(); - }, [dateTimeString]); + const handleTimestampQuery = useCallback(() => { + const raw = dateTimeString?.trim(); + if (false == raw) { + return; + } + const iso = raw.endsWith("Z") ? raw : `${raw}Z`; + const ts = Date.parse(iso); + if (false == Number.isFinite(ts)) { + return; + } + // First sync view hash (it may rewrite defaults), then set final timestamp. + updateViewHashParams(); + updateWindowUrlHashParams({timestamp: Math.floor(ts)}); + }, [dateTimeString]);
45-49: Guard Enter against IME composition; use imported event typePrevents premature submit during composition input.
- const handleKeyboardEnterPress = useCallback((ev: React.KeyboardEvent<HTMLInputElement>) => { - if ("Enter" === ev.key) { + const handleKeyboardEnterPress = useCallback((ev: KeyboardEvent<HTMLInputElement>) => { + // Avoid triggering while composing (IME) + // @ts-expect-error nativeEvent.isComposing exists at runtime + if ((false == (ev as any)?.nativeEvent?.isComposing) && "Enter" === ev.key) { handleTimestampQuery(); } }, [handleTimestampQuery]);
51-54: Trim input and follow param naming convention (ev for events)Prevents stray whitespace issues and matches repo style.
- const handleDateTimeInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { + const handleDateTimeInputChange = useCallback((ev: ChangeEvent<HTMLInputElement>) => { const {setDateTimeString} = useViewStore.getState(); - setDateTimeString(e.currentTarget.value); + setDateTimeString(ev.currentTarget.value.trim()); }, []);
58-82: A11y/UX: add step/placeholder/labels; disable icon buttons when input is disabledImproves usability, accessibility, and state consistency.
- <Tooltip title={"Jump to the nearest log event at / before this UTC time"}> + <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"} + title={"Timestamp to seek to in UTC"} type={"datetime-local"} value={dateTimeString} + placeholder={"YYYY-MM-DDThh:mm[:ss[.SSS]]"} + step={"0.001"} + aria-label={"UTC timestamp"} endDecorator={ <MenuBarIconButton + disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} + tooltipTitle={"Search by timestamp"} + tooltipPlacement={"bottom-start"} + aria-label={"Search by timestamp"} onClick={handleTimestampQuery} > <SearchIcon/> </MenuBarIconButton> } startDecorator={ <MenuBarIconButton + disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} + tooltipTitle={"Collapse timestamp query"} + tooltipPlacement={"bottom-start"} + aria-label={"Collapse timestamp query"} onClick={onInputCollapse} > <CollapseIcon/> </MenuBarIconButton> } onChange={handleDateTimeInputChange} onKeyDown={handleKeyboardEnterPress}/> </Tooltip>src/components/MenuBar/TimestampQueryContainer.tsx (1)
40-46: Add basic a11y wiring between toggle and regionExpose state and control relationship for assistive tech.
<MenuBarIconButton disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} tooltipTitle={"Search by timestamp"} - onClick={handleInputVisibilityToggle} + onClick={handleInputVisibilityToggle} + aria-label={isInputVisible ? "Collapse timestamp query" : "Open timestamp query"} + aria-expanded={isInputVisible} + aria-controls={"timestamp-query-region"} + aria-pressed={isInputVisible} > <CalendarTodayIcon/> </MenuBarIconButton> @@ - <div + <div className={`timestamp-query-input-wrapper ${isInputVisible ? "expanded" : ""}`} + id={"timestamp-query-region"} + role={"region"} + aria-label={"Timestamp query panel"} + aria-hidden={false == isInputVisible} > <TimestampQueryInput onInputCollapse={handleInputVisibilityToggle}/> </div>Also applies to: 50-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/MenuBar/TimestampQueryContainer.tsx(1 hunks)src/components/MenuBar/TimestampQueryInput.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/MenuBar/TimestampQueryContainer.tsxsrc/components/MenuBar/TimestampQueryInput.tsx
🧠 Learnings (4)
📚 Learning: 2024-09-28T02:32:08.882Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-09-28T02:32:08.882Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Applied to files:
src/components/MenuBar/TimestampQueryContainer.tsx
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
📚 Learning: 2024-09-28T01:15:34.533Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
📚 Learning: 2025-08-30T15:09:33.710Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#359
File: src/components/MenuBar/TimestampQueryBox.tsx:59-61
Timestamp: 2025-08-30T15:09:33.710Z
Learning: In the yscope-log-viewer application, the timestamp query functionality jumps to the last log event at the specified UTC time, not the first log event at or after that time.
Applied to files:
src/components/MenuBar/TimestampQueryInput.tsx
🧬 Code graph analysis (1)
src/components/MenuBar/TimestampQueryContainer.tsx (1)
src/typings/states.ts (1)
UI_ELEMENT(105-105)
Comment on lines
+41
to
+44
| disabled={isDisabled(uiState, UI_ELEMENT.NAVIGATION_BAR)} | ||
| tooltipTitle={"Search by timestamp"} | ||
| onClick={handleInputVisibilityToggle} | ||
| > |
There was a problem hiding this comment.
🧹 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.
Comment on lines
+27
to
+34
| /** | ||
| * 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 | ||
| */ |
There was a problem hiding this comment.
🧹 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.
junhaoliao
approved these changes
Sep 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces a new
TimestampQueryBoxcomponent to theMenuBar, allowing users to input date and time in a picker and perform queries based on that timestamp by clicking the search button. The changes include the addition of the component, its styling, and its integration into the existingMenuBar.New
TimestampQueryBoxComponent:Implementation of
TimestampQueryBox: Added a new React component insrc/components/MenuBar/TimestampQueryBox.tsxthat renders an input field for UTC datetime and a search button. The component updates the application's state and URL hash parameters when the user presses "Enter" or clicks the search button.Styling for
TimestampQueryBox: Added CSS rules insrc/components/MenuBar/TimestampQueryBox.cssto style the component, including flexbox layout for alignment and input field styling.Integration into
MenuBar:Importing
TimestampQueryBox: Imported the new component insrc/components/MenuBar/index.tsxfor integration into theMenuBar.Adding
TimestampQueryBoxto theMenuBar: Integrated theTimestampQueryBoxcomponent into theMenuBarlayout, placing it between vertical dividers for visual separation.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Style