feat: Add timezone select UI to the status bar. #336
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
## Walkthrough
This change introduces timezone selection functionality to the application. It adds a new `TimezoneSelect` component in the status bar, updates global state and URL hash management to support a `timezone` parameter, and provides supporting types, constants, and utility functions for timezone handling. Related UI and store logic are updated accordingly.
## Changes
| File(s) | Change Summary |
|--------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| src/components/AppController.tsx | Modified `updateViewHashParams` to handle the new `timezone` URL hash parameter and update state accordingly. |
| src/components/StatusBar/TimezoneSelect/index.tsx,<br>src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.tsx | Added new `TimezoneSelect` dropdown component and supporting `TimezoneCategoryChip` for timezone category display. Integrates with state and URL hash. |
| src/components/StatusBar/TimezoneSelect/index.css,<br>src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.css | Added new CSS files for styling the timezone selector and category chip. |
| src/components/StatusBar/index.tsx | Inserted the new `TimezoneSelect` component into the status bar layout. |
| src/components/StatusBar/LogLevelSelect/index.css,<br>src/main.css | Updated list box max-height logic to use a new CSS variable; defined the variable in `main.css`. |
| src/stores/viewStore/createViewFormattingSlice.ts,<br>src/stores/viewStore/types.ts | Added `timezoneName` state and `updateTimezoneName` method to the view formatting slice and its interface. |
| src/typings/date.ts | Introduced timezone-related types, constants, enums, and utility functions. |
| src/typings/states.ts | Added `TIMEZONE_SETTER` to `UI_ELEMENT` enum and updated UI state grid to control its enabled/disabled status. |
| src/typings/url.ts | Added `TIMEZONE` to `HASH_PARAM_NAMES` and extended `UrlHashParams` interface. |
| src/utils/date.ts | Added utility function `getUtcOffsetFrom` for parsing UTC offset strings to minutes. |
| src/utils/url.ts | Extended `URL_HASH_PARAMS_DEFAULT` and parsing logic to include the `timezone` parameter. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant StatusBar
participant TimezoneSelect
participant viewStore
participant URLHash
participant LogFileManagerProxy
User->>TimezoneSelect: Selects a timezone from dropdown
TimezoneSelect->>viewStore: updateTimezoneName(newTimezone)
viewStore->>viewStore: Set fast loading state, update timezoneName
viewStore->>LogFileManagerProxy: loadPage with new timezone
LogFileManagerProxy-->>viewStore: Returns new page data
viewStore->>viewStore: Update page data
TimezoneSelect->>URLHash: Update URL hash with timezone paramPossibly related PRs
Suggested reviewers
Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: davemarco |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
src/components/AppController.tsx(1 hunks)src/components/StatusBar/LogLevelSelect/index.css(1 hunks)src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.css(1 hunks)src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.tsx(1 hunks)src/components/StatusBar/TimezoneSelect/index.css(1 hunks)src/components/StatusBar/TimezoneSelect/index.tsx(1 hunks)src/components/StatusBar/index.tsx(2 hunks)src/main.css(1 hunks)src/stores/viewStore/createViewFormattingSlice.ts(3 hunks)src/stores/viewStore/types.ts(1 hunks)src/typings/date.ts(1 hunks)src/typings/states.ts(5 hunks)src/typings/url.ts(2 hunks)src/utils/date.ts(1 hunks)src/utils/url.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/components/StatusBar/index.tsxsrc/components/AppController.tsxsrc/typings/states.tssrc/utils/url.tssrc/typings/url.tssrc/stores/viewStore/types.tssrc/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.tsxsrc/stores/viewStore/createViewFormattingSlice.tssrc/utils/date.tssrc/components/StatusBar/TimezoneSelect/index.tsxsrc/typings/date.ts
🧠 Learnings (12)
📓 Common learnings
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#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: 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.
src/components/StatusBar/index.tsx (4)
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.
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: 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.
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.
src/components/StatusBar/LogLevelSelect/index.css (8)
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.
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.
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.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/ResizeHandle.css:0-0
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The CSS property names are from JoyUI and cannot be easily overridden.
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.
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.
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.
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.
src/main.css (4)
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/ResizeHandle.css:0-0
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The CSS property names are from JoyUI and cannot be easily overridden.
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.
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.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:23-26
Timestamp: 2024-10-08T15:52:50.753Z
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.
src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.css (1)
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.
src/components/AppController.tsx (2)
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.
src/utils/url.ts (1)
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.
src/typings/url.ts (2)
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/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/types.ts (1)
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#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/components/StatusBar/TimezoneSelect/index.css (7)
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.
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.
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.
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.
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.
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.
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:02:39.092Z
Learning: When overriding JoyUI styles, `!important` may be necessary to ensure custom styles take precedence.
src/stores/viewStore/createViewFormattingSlice.ts (3)
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.
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#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.
src/components/StatusBar/TimezoneSelect/index.tsx (4)
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: 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.
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.
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.
🧬 Code Graph Analysis (4)
src/components/AppController.tsx (1)
src/utils/url.ts (1)
getWindowUrlHashParams(334-334)
src/utils/url.ts (1)
src/typings/url.ts (1)
HASH_PARAM_NAMES(39-39)
src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.tsx (1)
src/typings/date.ts (1)
TIMEZONE_CATEGORY(106-106)
src/components/StatusBar/TimezoneSelect/index.tsx (4)
src/typings/date.ts (6)
DEFAULT_TIMEZONE_NAME(101-101)BROWSER_TIMEZONE_NAME(100-100)LOGGER_TIMEZONE_NAME(105-105)UTC_TIMEZONE_OFFSET_NAMES(107-107)INTL_SUPPORTED_TIMEZONE_NAMES(103-103)getTimezoneCategory(102-102)src/utils/url.ts (1)
updateWindowUrlHashParams(339-339)src/typings/url.ts (1)
HASH_PARAM_NAMES(39-39)src/typings/states.ts (1)
UI_ELEMENT(110-110)
🪛 Biome (1.9.4)
src/stores/viewStore/createViewFormattingSlice.ts
[error] 67-67: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
src/utils/date.ts
[error] 13-13: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🔇 Additional comments (18)
src/main.css (1)
26-28: Centralized list-box height variable looks goodMoving the
calc(100vh − menu − status)expression into a root-level custom property keeps all dropdowns in sync and simplifies future tweaks.
No further concerns.src/components/StatusBar/LogLevelSelect/index.css (1)
17-18: Good reuse of the new --ylv-list-box-max-height variableSwitching to the shared custom property removes duplicate calculations and guarantees both dropdowns stay aligned. Nice cleanup.
src/typings/url.ts (1)
10-15: Verify complete propagation of the newtimezonehash parameterAdding the enum entry and type field is the right first step. Please double-check that:
URL_HASH_PARAMS_DEFAULTnow defines a sane initial value (e.g.,""or system TZ).- All call-sites that read / write hash params include the new key to avoid silent drops.
A quick
rg '"timezone"'search should surface any gaps.Also applies to: 27-28
src/components/AppController.tsx (1)
27-32: LGTM! Timezone parameter integration follows the established pattern.The implementation correctly integrates the timezone parameter into the URL hash parameter handling, following the same pattern as other parameters like
isPrettifiedandlogEventNum.src/typings/states.ts (2)
42-42: LGTM! New UI element added to enum.The
TIMEZONE_SETTERelement is properly added to theUI_ELEMENTenum.
69-105: LGTM! UI state grid updated with logical timezone setter states.The timezone setter is correctly disabled during loading states and enabled when the application is ready, following the same logical pattern as other UI elements in the grid.
src/utils/url.ts (2)
28-28: LGTM! Timezone parameter added to default hash parameters.The timezone parameter is correctly added to the default hash parameters with an appropriate empty string default value.
184-185: LGTM! Timezone parameter parsing follows established pattern.The timezone parameter parsing correctly handles the string value and follows the same pattern as other string parameters like
QUERY_STRING.src/stores/viewStore/types.ts (2)
35-35: LGTM! Timezone state property added to formatting values.The
timezoneNameproperty is correctly added to theViewFormattingValuesinterface with appropriate string typing.
40-40: LGTM! Timezone action added to formatting actions.The
updateTimezoneNameaction is correctly added to theViewFormattingActionsinterface with the appropriate function signature.src/components/StatusBar/TimezoneSelect/index.css (1)
1-14: LGTM! CSS follows established patterns for JoyUI style overrides.The CSS properly overrides JoyUI component styles using
!importantflags, which is acceptable for this codebase. The use of CSS custom properties and variables aligns with the project's styling approach.src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.tsx (1)
46-66: LGTM! Component follows coding guidelines and best practices.The component correctly uses
false === isDefaultinstead of!isDefault, following the project's coding guidelines. The conditional rendering logic is clear and the component is well-structured with proper TypeScript typing.src/stores/viewStore/createViewFormattingSlice.ts (1)
79-106: LGTM! Async operation follows established patterns.The async operation correctly handles loading page data with proper error handling, following the same pattern as the existing
updateIsPrettifiedmethod.src/components/StatusBar/TimezoneSelect/index.tsx (2)
40-72: LGTM! Component follows React best practices.The component properly uses React hooks with appropriate dependencies, implements proper error handling for the non-clearable dropdown, and efficiently manages state with memoization and callbacks.
76-111: LGTM! Autocomplete configuration is comprehensive and appropriate.The component configuration includes proper styling, event handlers, and UI behavior. The dynamic input width calculation and timezone categorization enhance the user experience.
src/typings/date.ts (3)
1-11: LGTM! Well-documented type definition.The
Timezonetype union appropriately covers both numeric UTC offsets and string timezone identifiers, with clear documentation explaining the different formats.
12-61: LGTM! Comprehensive timezone constants and data.The constants provide good coverage of timezone options, including UTC offsets and browser-supported timezone names. The use of
Intl.supportedValuesOf("timeZone")ensures compatibility with the browser's capabilities.
63-96: LGTM! Well-structured utility functions.The utility functions are focused, well-documented, and provide clear categorization logic for different timezone types. The enum-based categorization makes the code more maintainable.
Comment on lines
91
to
95
| <Divider orientation={"vertical"}/> | ||
|
|
||
| <TimezoneSelect/> | ||
| <LogLevelSelect/> | ||
| <Divider orientation={"vertical"}/> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add a divider before LogLevelSelect for visual symmetry
The status-bar controls are currently separated by vertical dividers, except between the newly-inserted TimezoneSelect and the existing LogLevelSelect. Adding one keeps the rhythm consistent.
- <TimezoneSelect/>
- <LogLevelSelect/>
+ <TimezoneSelect/>
+ <Divider orientation={"vertical"}/>
+ <LogLevelSelect/>📝 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
| <Divider orientation={"vertical"}/> | |
| <TimezoneSelect/> | |
| <LogLevelSelect/> | |
| <Divider orientation={"vertical"}/> | |
| <Divider orientation={"vertical"}/> | |
| <TimezoneSelect/> | |
| <Divider orientation={"vertical"}/> | |
| <LogLevelSelect/> | |
| <Divider orientation={"vertical"}/> |
🤖 Prompt for AI Agents
In src/components/StatusBar/index.tsx between lines 91 and 95, add a vertical
Divider component before the LogLevelSelect component to maintain consistent
visual separation between controls. This will ensure the spacing and rhythm of
the status-bar controls remain uniform by placing a Divider both before and
after LogLevelSelect.
Comment on lines
+27
to
28
| import TimezoneSelect from "./TimezoneSelect"; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider lazy-loading the new TimezoneSelect import
TimezoneSelect is only used inside the status bar and most users may never open its listbox. Wrapping the import with React.lazy() + Suspense would shave a few KB from the initial bundle without UX impact.
-import TimezoneSelect from "./TimezoneSelect";
+const TimezoneSelect = React.lazy(() => import("./TimezoneSelect"));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/StatusBar/index.tsx around lines 27 to 28, the TimezoneSelect
component is imported normally but is only used inside the status bar and may
not be needed immediately. To reduce the initial bundle size, change the import
to use React.lazy() for lazy loading and wrap the usage of TimezoneSelect in a
Suspense component with a fallback UI. This will defer loading TimezoneSelect
until it is actually rendered, improving performance without affecting user
experience.
Comment on lines
+1
to
+8
| .timezone-category-chip { | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| .timezone-category-chip-default-timezone { | ||
| /* Disable `Chip`'s background style. */ | ||
| background-color: initial !important; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Nit: make the chip truly non-interactive when disabled
If a category ever needs a disabled state, consider adding pointer-events: none; alongside the existing background override to prevent misleading hover cues.
No action required for this PR.
🤖 Prompt for AI Agents
In src/components/StatusBar/TimezoneSelect/TimezoneCategoryChip.css lines 1 to
8, to make the chip truly non-interactive when disabled, add pointer-events:
none; to the .timezone-category-chip-default-timezone class alongside the
background-color override. This will prevent hover and click interactions,
ensuring the disabled state is clear and not misleading.
Comment on lines
+10
to
+17
| const [hours, minutes] = timezoneName | ||
| .replace("UTC", "") | ||
| .split(":") | ||
| .map((part) => parseInt(part, 10)); | ||
|
|
||
| return ( | ||
| (hours || 0) * MINUTES_IN_HOUR | ||
| ) + (minutes || 0); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use Number.parseInt instead of global parseInt.
The static analysis tool correctly identifies that Number.parseInt should be used instead of the global parseInt for consistency with ES2015 standards.
- .map((part) => parseInt(part, 10));
+ .map((part) => Number.parseInt(part, 10));📝 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
| const [hours, minutes] = timezoneName | |
| .replace("UTC", "") | |
| .split(":") | |
| .map((part) => parseInt(part, 10)); | |
| return ( | |
| (hours || 0) * MINUTES_IN_HOUR | |
| ) + (minutes || 0); | |
| const [hours, minutes] = timezoneName | |
| .replace("UTC", "") | |
| .split(":") | |
| .map((part) => Number.parseInt(part, 10)); | |
| return ( | |
| (hours || 0) * MINUTES_IN_HOUR | |
| ) + (minutes || 0); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🤖 Prompt for AI Agents
In src/utils/date.ts around lines 10 to 17, replace the use of the global
parseInt function with Number.parseInt for parsing the timezone parts. This
means changing the map callback to use Number.parseInt(part, 10) instead of
parseInt(part, 10) to align with ES2015 standards and improve consistency.
Comment on lines
+9
to
+18
| const getUtcOffsetFrom = (timezoneName: string): number => { | ||
| const [hours, minutes] = timezoneName | ||
| .replace("UTC", "") | ||
| .split(":") | ||
| .map((part) => parseInt(part, 10)); | ||
|
|
||
| return ( | ||
| (hours || 0) * MINUTES_IN_HOUR | ||
| ) + (minutes || 0); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding input validation for robustness.
The function works correctly for valid timezone strings, but could be more robust with input validation to handle edge cases.
Consider adding validation to ensure the input string starts with "UTC" and has the expected format:
const getUtcOffsetFrom = (timezoneName: string): number => {
+ if (!timezoneName.startsWith("UTC")) {
+ return 0;
+ }
+
const [hours, minutes] = timezoneName
.replace("UTC", "")
.split(":")
- .map((part) => parseInt(part, 10));
+ .map((part) => Number.parseInt(part, 10));
return (
(hours || 0) * MINUTES_IN_HOUR
) + (minutes || 0);
};📝 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
| const getUtcOffsetFrom = (timezoneName: string): number => { | |
| const [hours, minutes] = timezoneName | |
| .replace("UTC", "") | |
| .split(":") | |
| .map((part) => parseInt(part, 10)); | |
| return ( | |
| (hours || 0) * MINUTES_IN_HOUR | |
| ) + (minutes || 0); | |
| }; | |
| const getUtcOffsetFrom = (timezoneName: string): number => { | |
| if (!timezoneName.startsWith("UTC")) { | |
| return 0; | |
| } | |
| const [hours, minutes] = timezoneName | |
| .replace("UTC", "") | |
| .split(":") | |
| .map((part) => Number.parseInt(part, 10)); | |
| return ( | |
| (hours || 0) * MINUTES_IN_HOUR | |
| ) + (minutes || 0); | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🤖 Prompt for AI Agents
In src/utils/date.ts around lines 9 to 18, the getUtcOffsetFrom function lacks
input validation for the timezoneName parameter. Add checks to ensure the input
string starts with "UTC" and follows the expected format before parsing. If the
input is invalid, handle it gracefully by returning a default value or throwing
an error to prevent incorrect calculations.
Comment on lines
+65
to
+78
| updateTimezoneName: (newTimezoneName: string) => { | ||
| if ("" === newTimezoneName) { | ||
| newTimezoneName = DEFAULT_TIMEZONE_NAME; | ||
| } | ||
|
|
||
| const {numEvents} = useLogFileStore.getState(); | ||
| if (0 === numEvents) { | ||
| return; | ||
| } | ||
|
|
||
| const {timezoneName} = get(); | ||
| if (newTimezoneName === timezoneName) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a local variable instead of reassigning the parameter.
The method reassigns the newTimezoneName parameter, which can be confusing. Use a local variable for better clarity.
updateTimezoneName: (newTimezoneName: string) => {
- if ("" === newTimezoneName) {
- newTimezoneName = DEFAULT_TIMEZONE_NAME;
- }
+ const timezoneName = "" === newTimezoneName ? DEFAULT_TIMEZONE_NAME : newTimezoneName;
const {numEvents} = useLogFileStore.getState();
if (0 === numEvents) {
return;
}
- const {timezoneName} = get();
- if (newTimezoneName === timezoneName) {
+ const {timezoneName: currentTimezoneName} = get();
+ if (timezoneName === currentTimezoneName) {
return;
}📝 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
| updateTimezoneName: (newTimezoneName: string) => { | |
| if ("" === newTimezoneName) { | |
| newTimezoneName = DEFAULT_TIMEZONE_NAME; | |
| } | |
| const {numEvents} = useLogFileStore.getState(); | |
| if (0 === numEvents) { | |
| return; | |
| } | |
| const {timezoneName} = get(); | |
| if (newTimezoneName === timezoneName) { | |
| return; | |
| } | |
| updateTimezoneName: (newTimezoneName: string) => { | |
| const timezoneName = "" === newTimezoneName ? DEFAULT_TIMEZONE_NAME : newTimezoneName; | |
| const {numEvents} = useLogFileStore.getState(); | |
| if (0 === numEvents) { | |
| return; | |
| } | |
| const {timezoneName: currentTimezoneName} = get(); | |
| if (timezoneName === currentTimezoneName) { | |
| return; | |
| } | |
| // …rest of method… | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
🤖 Prompt for AI Agents
In src/stores/viewStore/createViewFormattingSlice.ts around lines 65 to 78, the
updateTimezoneName method reassigns the parameter newTimezoneName, which reduces
code clarity. To fix this, declare a new local variable initialized with
newTimezoneName and use that variable for any modifications instead of
reassigning the parameter directly.
hoophalab
requested review from
junhaoliao and
davemarco
and removed request for
junhaoliao
junhaoliao
reviewed
Jul 3, 2025
| const {logFileManagerProxy} = useLogFileManagerStore.getState(); | ||
| const {isPrettified, updatePageData} = get(); | ||
|
|
||
| // await logFileManagerProxy.setTimezone(newTimezoneName); |
junhaoliao
requested changes
Jul 3, 2025
|
I have some higher level questions:
|
|
|
@davemarco @hoophalab
or maybe simply
|
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 PR merges the time select UI from the dev branch. Only the UI components are merged at this point; the decoding feature will be added in a subsequent PR.
This PR exceeds +400 limit, but it should be fine as most code is from the dev branch.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes