refactor: Centralize hash parameter handling (fixes #342, #348); Simplify line number updates (fixes #346). #351
Conversation
… to `setIsPrettified`; remove `setPageData` and refactor `updatePageData`.
…teQueryHashParams` to urlHash
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@hoophalab please carefully check the following two commits; I've changed a bunch of files and it could be tedious to review. The rest of the commits should be bug fixes. |
# Conflicts: # src/components/StatusBar/index.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
src/components/AppController.tsx(4 hunks)src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx(2 hunks)src/components/Editor/index.tsx(3 hunks)src/components/StatusBar/index.tsx(2 hunks)src/stores/logFileStore.ts(3 hunks)src/stores/viewStore/createViewEventSlice.ts(1 hunks)src/stores/viewStore/createViewFilterSlice.ts(1 hunks)src/stores/viewStore/createViewFormattingSlice.ts(1 hunks)src/stores/viewStore/createViewPageSlice.ts(2 hunks)src/stores/viewStore/types.ts(2 hunks)src/utils/url/index.ts(1 hunks)src/utils/url/urlHash.ts(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/utils/url/index.tssrc/stores/viewStore/createViewFilterSlice.tssrc/components/StatusBar/index.tsxsrc/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsxsrc/stores/viewStore/createViewEventSlice.tssrc/stores/viewStore/types.tssrc/stores/viewStore/createViewFormattingSlice.tssrc/utils/url/urlHash.tssrc/stores/logFileStore.tssrc/components/Editor/index.tsxsrc/stores/viewStore/createViewPageSlice.tssrc/components/AppController.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-09-25T21:11:48.621Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
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.
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.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
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.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
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.
src/utils/url/index.ts (5)
Learnt from: davemarco
PR: #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.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: Henry8192
PR: #173
File: src/contexts/UrlContextProvider.tsx:1-1
Timestamp: 2025-05-27T18:09:40.038Z
Learning: The UrlContextProvider in src/contexts/UrlContextProvider.tsx is planned to be eventually replaced and deleted, so refactoring efforts to reduce its file size should be avoided despite it being large enough to require eslint-disable max-lines.
Learnt from: junhaoliao
PR: #84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:37:56.597Z
Learning: In our TypeScript codebase with strong typing, variables are strictly typed and cannot be undefined where null is expected, so checking for undefined in null checks is unnecessary.
src/stores/viewStore/createViewFilterSlice.ts (3)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: Henry8192
PR: #224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to handleExportChunk when logExportManager is null is intentional, as the page is reloaded in this scenario anyway.
src/components/StatusBar/index.tsx (9)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: Henry8192
PR: #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: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: #305
File: src/components/Editor/index.tsx:277-277
Timestamp: 2025-06-20T16:31:32.284Z
Learning: Monaco Editor's editor.actions.findWithArgs action uses isCaseSensitive as the parameter name for case sensitivity control, not matchCase. The correct parameters for this action include: searchString, isCaseSensitive, isRegex, replaceString, preserveCase, findInSelection, and matchWholeWord.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx (10)
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx:99-105
Timestamp: 2024-11-14T04:34:08.409Z
Learning: In the React component ResultsGroup in src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx, it's acceptable to use array indices as keys when rendering Result components from the results array, since the results are inserted in order and items are not reordered or deleted.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: Henry8192
PR: #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: #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.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: Henry8192
PR: #107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:9-12
Timestamp: 2024-10-28T18:39:28.680Z
Learning: In the Result component (Result.tsx), if end < start, the for loop won't run and it will return nothing, so additional validation for matchRange indices is not necessary.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: Henry8192
PR: #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.
Learnt from: junhaoliao
PR: #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.
src/stores/viewStore/createViewEventSlice.ts (5)
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: #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.
src/stores/viewStore/types.ts (7)
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: davemarco
PR: #123
File: src/services/decoders/ClpIrDecoder.ts:42-42
Timestamp: 2024-11-18T01:35:35.812Z
Learning: The interface YscopeFormatterOptionsType is defined in src/typings/formatters.ts and is used to type the options passed to YscopeFormatter, ensuring type safety in constructor calls like new YscopeFormatter({ formatString: decoderOptions.formatString }).
src/stores/viewStore/createViewFormattingSlice.ts (6)
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: davemarco
PR: #123
File: src/services/decoders/ClpIrDecoder.ts:90-90
Timestamp: 2024-11-18T01:35:37.012Z
Learning: Within ClpIrDecoder.ts, in the setFormatterOptions method, options.formatString is already typed as a string, so explicit casting to string is unnecessary.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #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.
src/utils/url/urlHash.ts (6)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: zzxthehappiest
PR: #286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
src/stores/logFileStore.ts (12)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: Henry8192
PR: #80
File: new-log-viewer/src/services/LogFileManager.ts:297-313
Timestamp: 2024-10-09T02:08:56.053Z
Learning: In the #searchChunk method of LogFileManager.ts, checking typeof match.index === "number" correctly handles matches at index 0, ensuring matches at the beginning of the string are not skipped.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: Henry8192
PR: #224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to handleExportChunk when logExportManager is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: #96
File: new-log-viewer/webpack.common.js:66-66
Timestamp: 2024-10-18T00:59:50.690Z
Learning: In new-log-viewer/webpack.common.js, __dirname refers to the new-log-viewer directory, so paths using path.resolve(__dirname, ...) are relative to new-log-viewer.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: Henry8192
PR: #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.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
src/components/Editor/index.tsx (11)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: junhaoliao
PR: #305
File: src/components/Editor/index.tsx:277-277
Timestamp: 2025-06-20T16:31:32.284Z
Learning: Monaco Editor's editor.actions.findWithArgs action uses isCaseSensitive as the parameter name for case sensitivity control, not matchCase. The correct parameters for this action include: searchString, isCaseSensitive, isRegex, replaceString, preserveCase, findInSelection, and matchWholeWord.
Learnt from: zzxthehappiest
PR: #286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
src/stores/viewStore/createViewPageSlice.ts (9)
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: #286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use setX for functions that simply change the value of a state, and use updateX for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: Henry8192
PR: #224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to handleExportChunk when logExportManager is null is intentional, as the page is reloaded in this scenario anyway.
src/components/AppController.tsx (12)
Learnt from: zzxthehappiest
PR: #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.
Learnt from: Henry8192
PR: #173
File: src/contexts/UrlContextProvider.tsx:1-1
Timestamp: 2025-05-27T18:09:40.038Z
Learning: The UrlContextProvider in src/contexts/UrlContextProvider.tsx is planned to be eventually replaced and deleted, so refactoring efforts to reduce its file size should be avoided despite it being large enough to require eslint-disable max-lines.
Learnt from: junhaoliao
PR: #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.
Learnt from: junhaoliao
PR: #265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the actions.ts file, the PAGE_TOP and PAGE_BOTTOM actions are not intended to be handled in the getPageNumCursorArgs function.
Learnt from: junhaoliao
PR: #224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: zzxthehappiest
PR: #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.
Learnt from: Henry8192
PR: #224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to handleExportChunk when logExportManager is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: davemarco
PR: #76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the getPageNumCursorArgs function, variables newPageNum and anchor are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: zzxthehappiest
PR: #286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
🧬 Code Graph Analysis (4)
src/stores/viewStore/createViewFilterSlice.ts (1)
src/typings/states.ts (1)
UI_STATE(106-106)
src/components/StatusBar/index.tsx (4)
src/utils/actions.ts (1)
ACTION_NAME(107-107)src/utils/url/index.ts (1)
updateWindowUrlHashParams(384-384)src/typings/url.ts (1)
HASH_PARAM_NAMES(39-39)src/components/AppController.tsx (1)
updateViewHashParams(97-97)
src/utils/url/urlHash.ts (7)
src/typings/common.ts (1)
Nullable(6-6)src/typings/worker.ts (2)
CursorType(110-110)CURSOR_CODE(103-103)src/services/LogFileManager/index.ts (2)
numEvents(129-131)cursor(439-473)src/utils/url/index.ts (4)
updateWindowUrlHashParams(384-384)URL_HASH_PARAMS_DEFAULT(386-386)updateUrlIfEventOnPage(383-383)getWindowUrlHashParams(378-378)src/components/AppController.tsx (1)
updateViewHashParams(97-97)src/typings/states.ts (1)
UI_STATE(106-106)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
src/stores/viewStore/createViewPageSlice.ts (2)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(384-384)src/typings/states.ts (1)
UI_STATE(106-106)
🔇 Additional comments (20)
src/stores/viewStore/createViewFilterSlice.ts (1)
45-45: Good addition for UI state consistency.Explicitly setting the UI state to
READYafter updating page data ensures proper UI state transitions and aligns with the consistent pattern used across other similar functions in the codebase.src/components/StatusBar/index.tsx (1)
25-25: Well-structured refactoring to centralize URL hash synchronization.The addition of
updateViewHashParams()after updating URL hash parameters follows the new centralized pattern for managing URL-to-state synchronization. This improves maintainability by consolidating the logic in the utility module.Also applies to: 54-58
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx (1)
9-9: Consistent refactoring pattern applied correctly.The replacement of direct store state updates with the centralized
updateViewHashParams()utility follows the established pattern and improves code maintainability. The sequence of updating URL hash parameters followed by the utility call is correct.Also applies to: 44-45
src/stores/logFileStore.ts (1)
24-24: Well-integrated changes supporting the URL hash refactoring.The changes effectively integrate with the new URL hash parameter management system:
- The import update aligns with the centralized utility approach
- Using
updatePageDataand retrievinglogEventNumfrom current state follows the simplified store pattern- Explicit UI state transition to
READYmaintains consistency- Conditional
startQuerybased onupdateQueryHashParams()return value is a smart optimizationThese changes work cohesively to support the broader architectural refactoring.
Also applies to: 119-127, 152-152, 154-157
src/stores/viewStore/createViewEventSlice.ts (1)
20-27: LGTM! Clean separation of concerns.The simplification of this slice aligns well with the refactoring objectives. Moving side effects and URL synchronization logic out of the store slice improves maintainability and follows the established pattern where
setXmethods are used for simple state changes.src/stores/viewStore/types.ts (1)
16-19: LGTM! Type definitions correctly updated.The interface changes properly reflect the refactoring:
- Method renamings follow the established convention (
setXfor simple state changes)- Removal of
setPageDataaligns with the consolidation of page data updatesAlso applies to: 27-29, 37-39
src/components/Editor/index.tsx (1)
259-277: LGTM! Proper delegation of URL synchronization.The cursor position change handler correctly updates the URL parameters first, then delegates the synchronization to
updateViewHashParams().src/stores/viewStore/createViewFormattingSlice.ts (1)
20-27: LGTM! Consistent refactoring pattern.The simplification follows the same pattern as other view slices, removing side effects and creating a pure state setter. This improves testability and maintainability.
src/components/AppController.tsx (1)
29-35: LGTM! Clean delegation of hash parameter handling.The simplified
handleHashChangefunction properly delegates to the centralized utilities and conditionally starts queries based on the return value.src/stores/viewStore/createViewPageSlice.ts (4)
95-104: Function name aligns with Zustand store naming conventions.The rename from
setPageDatatoupdatePageDatacorrectly follows the established pattern whereupdateXfunctions perform additional logic beyond just setting state values, as seen with the URL hash parameter synchronization.
103-103: URL hash parameter update is correctly centralized.The consolidation of URL hash parameter updates into
updatePageDataimproves the separation of concerns and aligns with the broader refactoring to centralize URL synchronization logic.
146-146: UI state management correctly moved outside page data update.Moving
setUiState(UI_STATE.READY)outside ofupdatePageDataproperly separates page data updates from UI state management, making the code more maintainable and following the single responsibility principle.
97-99: No impact from property reordering
I searched across all.tsand.tsxfiles for any references that might rely on the ordering ofbeginLineNumToLogEventNum,numPages,pageNum, orlogEventNumand found none. Since Zustand’sset()call merges state atomically, changing the key order here does not affect functionality.src/utils/url/urlHash.ts (7)
31-78: Logic flow ingetCursorFromHashParamsis well-structured.The function correctly handles the priority order: prettification changes, timestamp navigation, then event number navigation. The early return pattern and null checks are appropriately implemented.
34-39: Proper handling of empty file state.The early return when
numEventsis 0 with URL parameter reset prevents unnecessary processing and maintains consistent state.
44-44: Event number clamping ensures data integrity.The clamping of
logEventNumwithin valid bounds (1 tonumEvents) prevents out-of-range navigation attempts and potential errors.
65-68: Optimization avoids unnecessary page loads.The check using
updateUrlIfEventOnPageto avoid requesting a new page when the target event is already visible is a good performance optimization.
84-105:updateViewHashParamsfunction is well-implemented.The function properly resets temporary URL parameters, processes the cursor, and handles the async page loading with appropriate error handling using the established
handleErrorWithNotificationpattern.
113-137:updateQueryHashParamsfunction efficiently tracks state changes.The function correctly synchronizes query parameters with store state and returns whether any modifications occurred, enabling conditional logic downstream.
127-137: Query parameter update logic uses non-standard logical operators.The code uses
||=operators which violate the coding guideline to preferfalse == <expression>rather than!<expression>.Apply this diff to align with coding guidelines:
- let isQueryModified = false; - isQueryModified ||= queryIsCaseSensitive !== currentQueryIsCaseSensitive; - setQueryIsCaseSensitive(queryIsCaseSensitive); - - isQueryModified ||= queryIsRegex !== currentQueryIsRegex; - setQueryIsRegex(queryIsRegex); - - isQueryModified ||= queryString !== currentQueryString; - setQueryString(queryString); + let isQueryModified = false; + if (false == (queryIsCaseSensitive === currentQueryIsCaseSensitive)) { + isQueryModified = true; + } + setQueryIsCaseSensitive(queryIsCaseSensitive); + + if (false == (queryIsRegex === currentQueryIsRegex)) { + isQueryModified = true; + } + setQueryIsRegex(queryIsRegex); + + if (false == (queryString === currentQueryString)) { + isQueryModified = true; + } + setQueryString(queryString);⛔ 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>`.Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#305 File: src/components/Editor/index.tsx:277-277 Timestamp: 2025-06-20T16:31:32.284Z Learning: Monaco Editor's `editor.actions.findWithArgs` action uses `isCaseSensitive` as the parameter name for case sensitivity control, not `matchCase`. The correct parameters for this action include: `searchString`, `isCaseSensitive`, `isRegex`, `replaceString`, `preserveCase`, `findInSelection`, and `matchWholeWord`.
…coInstance and logFileManager; fix y-scope#346.
Henry8192
changed the title
| }); | ||
| const {updateIsPrettified} = useViewStore.getState(); | ||
| updateIsPrettified(!isPrettified); | ||
| updateViewHashParams(); |
There was a problem hiding this comment.
I appreciate this change, but this removes the progress bar after clicking isPrettified button.
Can we rethink about where to set UI_STATE, we probably can either
- set UI_STATE to loading before calling
logFileManagerinsideupdateViewHashParams, or - use
loadPageByActioninupdateViewHashParams?
Personally, I feel we can do a hot fix in this pr: setUiState(FAST_LOADING) right here in this handle function and do the ui state cleanup later
There was a problem hiding this comment.
Let's do a hot fix and address the issue later.
There was a problem hiding this comment.
@coderabbitai can you create an issue on this?
There was a problem hiding this comment.
🧩 Analysis chain
I'll create an issue to track the UI state management cleanup that was discussed.
@hoophalab I've created an issue to track the UI state management cleanup that was discussed. The issue references this PR and the specific discussion, describes the problem with the progress bar, mentions the hot fix that was implemented, and includes the longer-term solutions you suggested.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the UI state management cleanup
gh issue create \
--title "Refactor UI state management in prettify toggle flow" \
--body "## Problem
After the refactoring in PR #351, the progress bar disappears when clicking the prettify button due to UI state management being scattered across different functions.
## Current Hot Fix
A hot fix was implemented by adding \`setUiState(UI_STATE.FAST_LOADING)\` directly in the StatusBar component's click handler, but this is not the ideal long-term solution.
## Suggested Solutions
Two approaches were discussed for a cleaner implementation:
1. Set \`UI_STATE\` to loading before calling \`logFileManager\` inside \`updateViewHashParams\`
2. Use \`loadPageByAction\` in \`updateViewHashParams\` instead of direct \`logFileManager\` calls
## Context
This issue arose during the refactoring that extracted individual hash parameter handling functions to centralized \`updateViewHashParams\`. The UI state management needs to be cleaned up to ensure consistent loading states across the application.
## Backlinks
- PR: https://github.com/y-scope/yscope-log-viewer/pull/351
- Discussion: https://github.com/y-scope/yscope-log-viewer/pull/351#discussion_r2229967738
Requested by: @hoophalab" \
--assignee hoophalab
Length of output: 1261
@hoophalab Perfect! I've successfully created issue #355 to track the UI state management cleanup: #355
The issue has been assigned to you and includes:
- Clear description of the progress bar problem after the prettify button refactoring
- Documentation of the current hot fix with
setUiState(UI_STATE.FAST_LOADING) - The two long-term solutions you suggested (setting UI_STATE before
logFileManagercall or usingloadPageByAction) - Proper backlinks to this PR and discussion thread
This will help track the follow-up work needed to clean up the UI state management after the refactoring is complete.
🧬 Code Graph Analysis Results
Relevant snippets for src/components/StatusBar/index.tsx
From src/utils/actions.ts (line 107)
ACTION_NAMEThis is an exported constant or enum used for action names, such as TOGGLE_PRETTIFY, referenced in the StatusBar component for button action handling.
From src/typings/url.ts (line 39)
HASH_PARAM_NAMESThis likely contains constants for URL hash parameter names, e.g., IS_PRETTIFIED, used when updating URL hash parameters in the StatusBar component.
From src/typings/states.ts (lines 105-106)
UI_ELEMENT
UI_STATEThese are enums or constants defining UI elements and UI states, used for checking disabled states and setting UI state in the StatusBar component.
From src/utils/url/urlHash.ts (line 141)
updateViewHashParamsFunction called after updating URL hash parameters to synchronize or update the view state accordingly.
From src/utils/url/index.ts (lines 375 and 384)
copyPermalinkToClipboard
updateWindowUrlHashParamsUtility functions used in the StatusBar component:
copyPermalinkToClipboard: Copies the current permalink to the clipboard.updateWindowUrlHashParams: Updates the URL hash parameters, e.g., toggling the prettify state.
From src/services/LogFileManager/index.ts (lines 129-131)
get numEvents () {
return this.#numEvents;
}Getter for the total number of log events, used in the StatusBar component to display the total event count.
These snippets provide definitions and implementations of constants, enums, and utility functions that are directly used in the StatusBar component for managing UI state, URL hash parameters, and clipboard operations.
🧠 Learnings used
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.
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.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
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.
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.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
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.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#347
File: src/utils/math.ts:47-56
Timestamp: 2025-07-24T21:28:56.838Z
Learning: In the y-scope/yscope-log-viewer codebase, Henry8192 prefers not to introduce local variables to avoid parameter reassignment in binary search implementations, considering the additional constants unnecessary.
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.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-09-25T21:11:48.621Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
…parameter isPrettified in the initial page load.
There was a problem hiding this comment.
we are taking an iterative approach towards the correct direction. We probably need to combine updateViewHashParameters with other flush-like methods in viewStore in the future
I confirm this change fixes #342 #346 #348.
However, there are still a few (comparatively minor) issues manifest in this PR: #355 #356.
i.e. the progress bar doesn't show in some cases, and the isPrettified parameter is discard in the initial load.
Validations:
Validated that the bugs mentioned in #342 #346 #348 no longer manifest.
|
For the PR title, how about: |
kirkrodrigues
changed the title
Description
This pull request refactors URL hash parameter handling and simplifies state management across multiple components. It removes redundant code, consolidates logic into reusable functions, and improves maintainability by streamlining the interaction between URL updates and application state. Key changes include the removal of
lineNumfromMonacoInstance, the introduction ofupdateViewHashParamsto centralize view-related URL updates, and the cleanup of unused imports and functions.Refactoring of URL hash parameter handling:
updateViewHashParamsfunction, simplifying how view-related URL hash parameters are handled.getCursorFromHashParams,updateViewHashParamsandupdateQueryHashParamsfunctions fromAppController.tsxtoutils/url/urlHash.ts.Result,Editor, andStatusBarto useupdateViewHashParamsfor consistency in updating view-related states.By refactoring this, #342 & #348 are fixed.
Simplification of
MonacoInstance:lineNumprop and its associated logic, including synchronization and cursor positioning, to streamline the component.Editorcomponent to handle cursor positioning directly usinggoToPositionAndCenterinstead of relying onlineNum.This fixes #346.
Cleanup and optimization:
updateLogEventNumandupdateIsPrettifiedwithsetLogEventNumandsetIsPrettifiedto align with the updated state management approach.Improvements to state management:
logFileStoreby replacingsetPageDatawithupdatePageData.updateQueryHashParamsinto the workflow.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Bug Fixes
New Features