Skip to content

feat: Support switching timezone of log event timestamp (resolves #207). #251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions docs/src/dev-guide/contributing-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ still be tested manually:
* Toggling tabbed panels in the sidebar
* Using keyboard shortcuts
* Toggling "Prettify" in both the status bar and the address bar
* Select different timezone in the "Timezone" dropdown menu in the status bar or URL

[gh-workflow-test]: https://github.com/y-scope/yscope-log-viewer/blob/main/.github/workflows/test.yaml
31 changes: 31 additions & 0 deletions src/components/StatusBar/TimezoneSelect/index.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* Since timezone is not multi select, and multi select has a different style in Joy components, so we have to manually
tune the style
*/
.timezone-select {
margin-right: 1px;
}

.timezone-select-render-value-box {
display: flex;
gap: 2px;
}

.timezone-select-render-value-box-label {
padding: 0 !important;

font-size: 0.95rem !important;
font-weight: 500 !important;
line-height: 1.5 !important;

background-color: initial !important;
border: none !important;
box-shadow: none !important;
}

.timezone-select-render-value-box-label-disabled {
color: #686f76 !important;
}

.timezone-select-listbox {
max-height: calc(100vh - var(--ylv-menu-bar-height) - var(--ylv-status-bar-height)) !important;
}
228 changes: 228 additions & 0 deletions src/components/StatusBar/TimezoneSelect/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
import React, {
useCallback,
useContext,
useEffect,
useRef,
useState,
} from "react";

import {SelectValue} from "@mui/base/useSelect";
import {
Box,
Chip,
ListDivider,
ListItemContent,
Option,
Select,
SelectOption,
} from "@mui/joy";

import KeyboardArrowUpIcon from "@mui/icons-material/KeyboardArrowUp";

import {
updateWindowUrlHashParams,
UrlContext,
} from "../../../contexts/UrlContextProvider";
import useUiStore from "../../../stores/uiStore";
import {UI_ELEMENT} from "../../../typings/states";
import {HASH_PARAM_NAMES} from "../../../typings/url";
import {isDisabled} from "../../../utils/states";

import "./index.css";


const LOGGER_TIMEZONE = "Logger Timezone";
const COMMON_TIMEZONES = [
"America/New_York",
"Asia/Shanghai",
"Asia/Tokyo",
"Australia/Sydney",
"Pacific/Honolulu",
"America/Los_Angeles",
"America/Chicago",
"America/Denver",
"Asia/Kolkata",
"Europe/Berlin",
"Europe/Moscow",
"Asia/Dubai",
"Asia/Singapore",
"Asia/Seoul",
"Pacific/Auckland",
];

/**
* Convert the timezone string to GMT +/- minutes
*
* @param tz
* @return The GMT +/- minutes shown before the timezone string
*/
const getLongOffsetOfTimezone = (tz: string): string => {
return new Intl.DateTimeFormat("default", {
timeZone: tz,
timeZoneName: "longOffset",
}).formatToParts()
.find((p) => "timeZoneName" === p.type)?.value ?? "Unknown timezone";
};
Comment on lines +59 to +65
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for invalid timezone strings.

The function doesn't handle invalid timezone strings gracefully. While there's a fallback to "Unknown timezone", it would be better to validate the timezone before attempting to use it.

const getLongOffsetOfTimezone = (tz: string): string => {
+    // Validate timezone before using it
+    try {
+        // Check if timezone is valid by attempting to use it
+        Intl.DateTimeFormat("default", { timeZone: tz });
+        
        return new Intl.DateTimeFormat("default", {
            timeZone: tz,
            timeZoneName: "longOffset",
        }).formatToParts()
            .find((p) => "timeZoneName" === p.type)?.value ?? "Unknown timezone";
+    } catch (error) {
+        console.warn(`Invalid timezone: ${tz}`);
+        return "Invalid timezone";
+    }
};
📝 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 getLongOffsetOfTimezone = (tz: string): string => {
return new Intl.DateTimeFormat("default", {
timeZone: tz,
timeZoneName: "longOffset",
}).formatToParts()
.find((p) => "timeZoneName" === p.type)?.value ?? "Unknown timezone";
};
const getLongOffsetOfTimezone = (tz: string): string => {
// Validate timezone before using it
try {
// Check if timezone is valid by attempting to use it
Intl.DateTimeFormat("default", { timeZone: tz });
return new Intl.DateTimeFormat("default", {
timeZone: tz,
timeZoneName: "longOffset",
}).formatToParts()
.find((p) => "timeZoneName" === p.type)?.value ?? "Unknown timezone";
} catch (error) {
console.warn(`Invalid timezone: ${tz}`);
return "Invalid timezone";
}
};


/**
* Render the selected timezone option in the status bar
*
* @param selected
* @return The selected timezone shown in the status bar
*/
const handleRenderValue = (selected: SelectValue<SelectOption<string>, false>) => (
<Box className={"timezone-select-render-value-box"}>
<Chip className={"timezone-select-render-value-box-label"}>Timezone</Chip>
<Chip>
{selected?.label}
</Chip>
</Box>
);

/**
* Render the timezone options in the dropdown menu
*
* @param value
* @param label
* @param onClick
* @param suffix
* @return An option box in the dropdown menu
*/
const renderTimezoneOption = (
value: string,
label: string,
onClick: React.MouseEventHandler,
suffix?: string
) => (
<Option
data-value={value}
key={value}
value={value}
onClick={onClick}
>
{LOGGER_TIMEZONE !== value &&
<ListItemContent>
(
{getLongOffsetOfTimezone(value)}
)
{" "}
{label}
{" "}
{suffix ?? ""}
</ListItemContent>}

{LOGGER_TIMEZONE === value &&
<ListItemContent>
{LOGGER_TIMEZONE}
</ListItemContent>}
</Option>
);

/**
* The timezone select dropdown menu, the selectable options can be classified as three types:
* - Default (use the origin timezone of the log events)
* - Browser Timezone (use the timezone that the browser is currently using)
* - Frequently-used Timezone
*
* @return A timezone select dropdown menu
*/
const TimezoneSelect = () => {
const uiState = useUiStore((state) => state.uiState);
const {logTimezone} = useContext(UrlContext);

const [browserTimezone, setBrowserTimezone] = useState<string | null>(null);
const [selectedTimezone, setSelectedTimezone] = useState<string | null>(null);

const logTimezoneRef = useRef<string | null>(logTimezone);

const disabled = isDisabled(uiState, UI_ELEMENT.TIMEZONE_SETTER);

useEffect(() => {
const tz = new Intl.DateTimeFormat().resolvedOptions().timeZone;
setBrowserTimezone(tz);
}, []);

useEffect(() => {
logTimezoneRef.current = logTimezone;
if (!disabled) {
setSelectedTimezone(logTimezone ?? LOGGER_TIMEZONE);
}
}, [disabled,
logTimezone]);

useEffect(() => {
if (selectedTimezone !== logTimezoneRef.current) {
const updatedTimezone = (LOGGER_TIMEZONE === selectedTimezone) ?
null :
selectedTimezone;

updateWindowUrlHashParams({
[HASH_PARAM_NAMES.LOG_TIMEZONE]: updatedTimezone,
});
}
}, [selectedTimezone]);

const handleOptionClick = useCallback((ev: React.MouseEvent) => {
const currentTarget = ev.currentTarget as HTMLElement;
const value = currentTarget.dataset.value ?? LOGGER_TIMEZONE;
setSelectedTimezone(value);
}, []);

return (
<Select
className={"timezone-select"}
disabled={disabled}
indicator={<KeyboardArrowUpIcon/>}
renderValue={handleRenderValue}
size={"sm"}
value={selectedTimezone}
variant={"soft"}
placeholder={
<Box className={"timezone-select-render-value-box"}>
<Chip
className={`timezone-select-render-value-box-label ${disabled ?
"timezone-select-render-value-box-label-disabled" :
""}`}
>
Timezone
</Chip>
</Box>
}
slotProps={{
listbox: {
className: "timezone-select-listbox",
placement: "top-end",
modifiers: [
{name: "equalWidth", enabled: false},
{name: "offset", enabled: false},
],
},
}}
onChange={(_, value) => {
if (value) {
setSelectedTimezone(value);
}
}}
>
{renderTimezoneOption(LOGGER_TIMEZONE, LOGGER_TIMEZONE, handleOptionClick)}

{browserTimezone &&
renderTimezoneOption(
browserTimezone,
browserTimezone,
handleOptionClick,
"(Browser Timezone)"
)}

<ListDivider
inset={"gutter"}
role={"separator"}/>

{COMMON_TIMEZONES.map(
(label) => renderTimezoneOption(label, label, handleOptionClick)
)}
</Select>
);
};

export default TimezoneSelect;
2 changes: 2 additions & 0 deletions src/components/StatusBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {ACTION_NAME} from "../../utils/actions";
import {isDisabled} from "../../utils/states";
import LogLevelSelect from "./LogLevelSelect";
import StatusBarToggleButton from "./StatusBarToggleButton";
import TimezoneSelect from "./TimezoneSelect";

import "./index.css";

Expand Down Expand Up @@ -67,6 +68,7 @@ const StatusBar = () => {
{/* This is left blank intentionally until status messages are implemented. */}
</Typography>

<TimezoneSelect/>
<Tooltip title={"Copy link to clipboard"}>
<span>
<Button
Expand Down
20 changes: 18 additions & 2 deletions src/contexts/AppController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@
* @param props.children
* @return
*/
const AppController = ({children}: StateContextProviderProps) => {

Check failure on line 85 in src/contexts/AppController.tsx

View workflow job for this annotation

GitHub Actions / lint-check

Arrow function has too many statements (21). Maximum allowed is 20
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the linting error for function size

The function now exceeds the maximum allowed statements (21 vs limit of 20) according to ESLint configuration.

Consider refactoring by extracting some functionality into separate helper functions to reduce the statement count within the main component function.

🧰 Tools
🪛 GitHub Check: lint-check

[failure] 85-85:
Arrow function has too many statements (21). Maximum allowed is 20

🪛 ESLint

[error] 85-214: Arrow function has too many statements (21). Maximum allowed is 20.

(max-statements)

🪛 GitHub Actions: lint

[error] 85-85: ESLint: Arrow function has too many statements (21). Maximum allowed is 20. (max-statements)

const {postPopUp} = useContext(NotificationContext);
const {filePath, isPrettified, logEventNum} = useContext(UrlContext);
const {filePath, isPrettified, logEventNum, logTimezone} = useContext(UrlContext);

// States
const beginLineNumToLogEventNum = useViewStore((state) => state.beginLineNumToLogEventNum);
Expand All @@ -93,12 +93,14 @@
const {logFileManagerProxy} = useLogFileManagerStore.getState();
const numEvents = useLogFileStore((state) => state.numEvents);
const setLogEventNum = useContextStore((state) => state.setLogEventNum);
const setLogTimezone = useViewStore((state) => state.updateLogTimezone);
const setUiState = useUiStore((state) => state.setUiState);
const setPostPopUp = useContextStore((state) => state.setPostPopUp);

// Refs
const isPrettifiedRef = useRef<boolean>(isPrettified ?? false);
const logEventNumRef = useRef(logEventNum);
const logTimezoneRef = useRef(logTimezone);

// Synchronize `logEventNumRef` with `logEventNum`.
useEffect(() => {
Expand All @@ -120,6 +122,15 @@
setIsPrettified,
]);

// Synchronize `logTimezoneRef` with `logTimezone`.
useEffect(() => {
logTimezoneRef.current = logTimezone;
setLogTimezone(logTimezoneRef.current);
}, [
logTimezone,
setLogTimezone,
]);

// On `logEventNum` update, clamp it then switch page if necessary or simply update the URL.
useEffect(() => {
if (0 === numEvents || URL_HASH_PARAMS_DEFAULT.logEventNum === logEventNum) {
Expand All @@ -144,7 +155,12 @@
setUiState(UI_STATE.FAST_LOADING);

(async () => {
const pageData = await logFileManagerProxy.loadPage(cursor, isPrettifiedRef.current);
const pageData = await logFileManagerProxy.loadPage(
cursor,
isPrettifiedRef.current,
logTimezoneRef.current,
);

useViewStore.getState().updatePageData(pageData);
})().catch((e: unknown) => {
postPopUp({
Expand Down
6 changes: 6 additions & 0 deletions src/contexts/UrlContextProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const URL_SEARCH_PARAMS_DEFAULT = Object.freeze({
const URL_HASH_PARAMS_DEFAULT = Object.freeze({
[HASH_PARAM_NAMES.IS_PRETTIFIED]: false,
[HASH_PARAM_NAMES.LOG_EVENT_NUM]: null,
[HASH_PARAM_NAMES.LOG_TIMEZONE]: null,
});

/**
Expand Down Expand Up @@ -223,6 +224,11 @@ const getWindowUrlHashParams = () => {
urlHashParams[HASH_PARAM_NAMES.IS_PRETTIFIED] = "true" === isPrettified;
}

const logTimezone = hashParams.get(HASH_PARAM_NAMES.LOG_TIMEZONE);
if (null !== logTimezone) {
urlHashParams[HASH_PARAM_NAMES.LOG_TIMEZONE] = logTimezone;
}

return urlHashParams;
};

Expand Down
Loading
Loading