Skip to content
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

feat(settings): Move settings from the modal into a tab panel. #186

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.sidebar-tab-panel {
padding: 0.75rem;
min-width: 0 !important;
padding: 0.75rem !important;
padding-right: 0.5rem !important;
}

.sidebar-tab-panel-container {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.theme-switch-toggle-button-group {
flex-wrap: wrap;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import DarkModeIcon from "@mui/icons-material/DarkMode";
import LightModeIcon from "@mui/icons-material/LightMode";
import SettingsBrightnessIcon from "@mui/icons-material/SettingsBrightness";

import {THEME_NAME} from "../../../typings/config";
import {THEME_NAME} from "../../../../../typings/config";

import "./ThemeSwitchFormField.css";


/**
Expand All @@ -29,6 +31,7 @@ const ThemeSwitchFormField = () => {
Theme
</FormLabel>
<ToggleButtonGroup
className={"theme-switch-toggle-button-group"}
size={"sm"}
value={mode as string}
onChange={(__, newValue) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.settings-tab-container {
overflow: hidden;
display: flex;
flex-direction: column;
gap: 0.75rem;

height: 100%;
}

.settings-form-fields-container {
overflow-y: auto;
display: flex;
flex-direction: column;
flex-grow: 1;
gap: 0.75rem;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import React, {
useCallback,
useContext,
} from "react";

import {
Box,
Button,
Divider,
FormControl,
FormHelperText,
FormLabel,
Input,
Link,
} from "@mui/joy";

import {NotificationContext} from "../../../../../contexts/NotificationContextProvider";
import {Nullable} from "../../../../../typings/common";
import {
CONFIG_KEY,
LOCAL_STORAGE_KEY,
} from "../../../../../typings/config";
import {LOG_LEVEL} from "../../../../../typings/logs";
import {DO_NOT_TIMEOUT_VALUE} from "../../../../../typings/notifications";
import {
TAB_DISPLAY_NAMES,
TAB_NAME,
} from "../../../../../typings/tab";
import {
getConfig,
setConfig,
} from "../../../../../utils/config";
import CustomTabPanel from "../CustomTabPanel";
import ThemeSwitchFormField from "./ThemeSwitchFormField";

import "./index.css";


const CONFIG_FORM_FIELDS = [
{
helperText: (
<span>
{"[JSON] Format string for formatting a JSON log event as plain text. See the "}
<Link
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/format-struct-logs-overview.html"}
level={"body-sm"}
rel={"noopener"}
target={"_blank"}
>
format string syntax docs
</Link>
{" or leave this blank to display the entire log event."}
</span>
),
initialValue: getConfig(CONFIG_KEY.DECODER_OPTIONS).formatString,
key: LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING,
label: "Decoder: Format string",
type: "text",
},
{
helperText: "[JSON] Key to extract the log level from.",
initialValue: getConfig(CONFIG_KEY.DECODER_OPTIONS).logLevelKey,
key: LOCAL_STORAGE_KEY.DECODER_OPTIONS_LOG_LEVEL_KEY,
label: "Decoder: Log level key",
type: "text",
},
{
helperText: "[JSON] Key to extract the log timestamp from.",
initialValue: getConfig(CONFIG_KEY.DECODER_OPTIONS).timestampKey,
key: LOCAL_STORAGE_KEY.DECODER_OPTIONS_TIMESTAMP_KEY,
label: "Decoder: Timestamp key",
type: "text",
},
{
helperText: "Number of log messages to display per page.",
initialValue: getConfig(CONFIG_KEY.PAGE_SIZE),
key: LOCAL_STORAGE_KEY.PAGE_SIZE,
label: "View: Page size",
type: "number",
},
];

/**
* Handles the reset event for the configuration form.
*
* @param ev
*/
const handleConfigFormReset = (ev: React.FormEvent) => {
ev.preventDefault();
window.localStorage.clear();
window.location.reload();
};
Comment on lines +88 to +92
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider selective localStorage clearing.

The current implementation clears all localStorage data. Consider clearing only configuration-related keys to preserve other application data.

-    window.localStorage.clear();
+    Object.values(LOCAL_STORAGE_KEY).forEach(key => {
+        window.localStorage.removeItem(key);
+    });
📝 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 handleConfigFormReset = (ev: React.FormEvent) => {
ev.preventDefault();
window.localStorage.clear();
window.location.reload();
};
const handleConfigFormReset = (ev: React.FormEvent) => {
ev.preventDefault();
Object.values(LOCAL_STORAGE_KEY).forEach(key => {
window.localStorage.removeItem(key);
});
window.location.reload();
};


/**
* Displays a setting tab panel for configurations.
*
* @return
*/
const SettingsTabPanel = () => {
const {postPopUp} = useContext(NotificationContext);

const handleConfigFormSubmit = useCallback(
(ev: React.FormEvent) => {
ev.preventDefault();
const formData = new FormData(ev.target as HTMLFormElement);
const getFormDataValue = (key: string) => formData.get(key) as string;

const formatString = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING);
const logLevelKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_LOG_LEVEL_KEY);
const timestampKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_TIMESTAMP_KEY);
const pageSize = getFormDataValue(LOCAL_STORAGE_KEY.PAGE_SIZE);

let error: Nullable<string> = null;
error ||= setConfig({
key: CONFIG_KEY.DECODER_OPTIONS,
value: {formatString, logLevelKey, timestampKey},
});
error ||= setConfig({
key: CONFIG_KEY.PAGE_SIZE,
value: Number(pageSize),
});

if (null !== error) {
postPopUp({
level: LOG_LEVEL.ERROR,
message: error,
timeoutMillis: DO_NOT_TIMEOUT_VALUE,
title: "Unable to apply config.",
});
} else {
window.location.reload();
}
},
[postPopUp],
);
Comment on lines +102 to +135
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 input validation before applying configuration.

Consider validating the form data before applying the configuration to prevent invalid values.

     const handleConfigFormSubmit = useCallback(
         (ev: React.FormEvent) => {
             ev.preventDefault();
             const formData = new FormData(ev.target as HTMLFormElement);
             const getFormDataValue = (key: string) => formData.get(key) as string;

             const formatString = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING);
             const logLevelKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_LOG_LEVEL_KEY);
             const timestampKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_TIMESTAMP_KEY);
             const pageSize = getFormDataValue(LOCAL_STORAGE_KEY.PAGE_SIZE);

+            // Validate page size
+            const pageSizeNum = Number(pageSize);
+            if (isNaN(pageSizeNum) || pageSizeNum <= 0) {
+                postPopUp({
+                    level: LOG_LEVEL.ERROR,
+                    message: "Page size must be a positive number",
+                    timeoutMillis: DO_NOT_TIMEOUT_VALUE,
+                    title: "Invalid configuration",
+                });
+                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
const handleConfigFormSubmit = useCallback(
(ev: React.FormEvent) => {
ev.preventDefault();
const formData = new FormData(ev.target as HTMLFormElement);
const getFormDataValue = (key: string) => formData.get(key) as string;
const formatString = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING);
const logLevelKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_LOG_LEVEL_KEY);
const timestampKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_TIMESTAMP_KEY);
const pageSize = getFormDataValue(LOCAL_STORAGE_KEY.PAGE_SIZE);
let error: Nullable<string> = null;
error ||= setConfig({
key: CONFIG_KEY.DECODER_OPTIONS,
value: {formatString, logLevelKey, timestampKey},
});
error ||= setConfig({
key: CONFIG_KEY.PAGE_SIZE,
value: Number(pageSize),
});
if (null !== error) {
postPopUp({
level: LOG_LEVEL.ERROR,
message: error,
timeoutMillis: DO_NOT_TIMEOUT_VALUE,
title: "Unable to apply config.",
});
} else {
window.location.reload();
}
},
[postPopUp],
);
const handleConfigFormSubmit = useCallback(
(ev: React.FormEvent) => {
ev.preventDefault();
const formData = new FormData(ev.target as HTMLFormElement);
const getFormDataValue = (key: string) => formData.get(key) as string;
const formatString = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING);
const logLevelKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_LOG_LEVEL_KEY);
const timestampKey = getFormDataValue(LOCAL_STORAGE_KEY.DECODER_OPTIONS_TIMESTAMP_KEY);
const pageSize = getFormDataValue(LOCAL_STORAGE_KEY.PAGE_SIZE);
// Validate page size
const pageSizeNum = Number(pageSize);
if (isNaN(pageSizeNum) || pageSizeNum <= 0) {
postPopUp({
level: LOG_LEVEL.ERROR,
message: "Page size must be a positive number",
timeoutMillis: DO_NOT_TIMEOUT_VALUE,
title: "Invalid configuration",
});
return;
}
let error: Nullable<string> = null;
error ||= setConfig({
key: CONFIG_KEY.DECODER_OPTIONS,
value: { formatString, logLevelKey, timestampKey },
});
error ||= setConfig({
key: CONFIG_KEY.PAGE_SIZE,
value: Number(pageSize),
});
if (null !== error) {
postPopUp({
level: LOG_LEVEL.ERROR,
message: error,
timeoutMillis: DO_NOT_TIMEOUT_VALUE,
title: "Unable to apply config.",
});
} else {
window.location.reload();
}
},
[postPopUp],
);


return (
<CustomTabPanel
tabName={TAB_NAME.SETTINGS}
title={TAB_DISPLAY_NAMES[TAB_NAME.SETTINGS]}
>
<form
className={"settings-tab-container"}
tabIndex={-1}
onReset={handleConfigFormReset}
onSubmit={handleConfigFormSubmit}
>
<Box className={"settings-form-fields-container"}>
<ThemeSwitchFormField/>
{CONFIG_FORM_FIELDS.map((field, index) => (
<FormControl key={index}>
<FormLabel>
{field.label}
</FormLabel>
<Input
defaultValue={field.initialValue}
name={field.key}
type={field.type}/>
<FormHelperText>
{field.helperText}
</FormHelperText>
</FormControl>
))}
</Box>
<Divider/>
<Button
color={"primary"}
type={"submit"}
>
Apply & Reload
</Button>
<Button
color={"neutral"}
type={"reset"}
>
Reset Default
</Button>
</form>
</CustomTabPanel>
);
};

export default SettingsTabPanel;
102 changes: 43 additions & 59 deletions src/components/CentralContainer/Sidebar/SidebarTabs/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
forwardRef,
Ref,
useContext,
} from "react";

Expand All @@ -17,9 +17,9 @@ import SettingsOutlinedIcon from "@mui/icons-material/SettingsOutlined";
import {StateContext} from "../../../../contexts/StateContextProvider";
import {TAB_NAME} from "../../../../typings/tab";
import {openInNewTab} from "../../../../utils/url";
import SettingsModal from "../../../modals/SettingsModal";
import FileInfoTabPanel from "./FileInfoTabPanel";
import SearchTabPanel from "./SearchTabPanel";
import SettingsTabPanel from "./SettingsTabPanel";
import TabButton from "./TabButton";

import "./index.css";
Expand All @@ -39,84 +39,68 @@ const TABS_INFO_LIST: Readonly<Array<{
]);

interface SidebarTabsProps {
activeTabName: TAB_NAME;
onActiveTabNameChange: (newValue: TAB_NAME) => void;
ref: Ref<HTMLDivElement>;
}

/**
* Displays a set of tabs in a vertical orientation.
*
* @param tabListRef Reference object used to access the TabList DOM element.
* @param props
* @param props.ref Reference object used to access the TabList DOM element.
* @return
*/
const SidebarTabs = forwardRef<HTMLDivElement, SidebarTabsProps>((
{
activeTabName,
onActiveTabNameChange,
},
tabListRef
) => {
const {isSettingsModalOpen, setIsSettingsModalOpen} = useContext(StateContext);

const handleSettingsModalClose = () => {
setIsSettingsModalOpen(false);
};
const SidebarTabs = ({
ref,
}: SidebarTabsProps) => {
Comment on lines +52 to +54
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Restore forwardRef wrapper.

Based on the retrieved learnings, this component should be wrapped with React.forwardRef. The current implementation might break ref forwarding functionality.

Apply this diff to restore the forwardRef wrapper:

-const SidebarTabs = ({
-    ref,
-}: SidebarTabsProps) => {
+const SidebarTabs = forwardRef<HTMLDivElement, SidebarTabsProps>(({}, ref) => {
📝 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 SidebarTabs = ({
ref,
}: SidebarTabsProps) => {
-const SidebarTabs = ({
- ref,
-}: SidebarTabsProps) => {
+const SidebarTabs = forwardRef<HTMLDivElement, SidebarTabsProps>(({}, ref) => {

const {activeTabName, changeActiveTabName} = useContext(StateContext);

const handleTabButtonClick = (tabName: TAB_NAME) => {
switch (tabName) {
case TAB_NAME.SETTINGS:
setIsSettingsModalOpen(true);
break;
case TAB_NAME.DOCUMENTATION:
openInNewTab(DOCUMENTATION_URL);
break;
default:
onActiveTabNameChange(tabName);
changeActiveTabName(tabName);
}
};
Comment on lines 57 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 security attributes for external links.

When opening documentation in a new tab, consider adding security-related attributes.

     case TAB_NAME.DOCUMENTATION:
-        openInNewTab(DOCUMENTATION_URL);
+        openInNewTab(DOCUMENTATION_URL, { rel: 'noopener noreferrer' });
         break;
📝 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 handleTabButtonClick = (tabName: TAB_NAME) => {
switch (tabName) {
case TAB_NAME.SETTINGS:
setIsSettingsModalOpen(true);
break;
case TAB_NAME.DOCUMENTATION:
openInNewTab(DOCUMENTATION_URL);
break;
default:
onActiveTabNameChange(tabName);
changeActiveTabName(tabName);
}
};
const handleTabButtonClick = (tabName: TAB_NAME) => {
switch (tabName) {
case TAB_NAME.DOCUMENTATION:
openInNewTab(DOCUMENTATION_URL, { rel: 'noopener noreferrer' });
break;
default:
changeActiveTabName(tabName);
}
};


return (
<>
<Tabs
className={"sidebar-tabs"}
orientation={"vertical"}
value={activeTabName}
variant={"plain"}
<Tabs
className={"sidebar-tabs"}
orientation={"vertical"}
value={activeTabName}
variant={"plain"}
>
<TabList
ref={ref}
size={"lg"}
>
<TabList
ref={tabListRef}
size={"lg"}
>
{TABS_INFO_LIST.map(({tabName, Icon}) => (
<TabButton
Icon={Icon}
key={tabName}
tabName={tabName}
onTabButtonClick={handleTabButtonClick}/>
))}

{/* Forces the help and settings tabs to the bottom of the sidebar. */}
<div className={"sidebar-tab-list-spacing"}/>

<TabButton
Icon={HelpOutlineIcon}
tabName={TAB_NAME.DOCUMENTATION}
onTabButtonClick={handleTabButtonClick}/>

{TABS_INFO_LIST.map(({tabName, Icon}) => (
<TabButton
Icon={SettingsOutlinedIcon}
tabName={TAB_NAME.SETTINGS}
Icon={Icon}
key={tabName}
tabName={tabName}
onTabButtonClick={handleTabButtonClick}/>
</TabList>
<FileInfoTabPanel/>
<SearchTabPanel/>
</Tabs>
<SettingsModal
isOpen={isSettingsModalOpen}
onClose={handleSettingsModalClose}/>
</>
))}

{/* Forces the help and settings tabs to the bottom of the sidebar. */}
<div className={"sidebar-tab-list-spacing"}/>

<TabButton
Icon={HelpOutlineIcon}
tabName={TAB_NAME.DOCUMENTATION}
onTabButtonClick={handleTabButtonClick}/>

<TabButton
Icon={SettingsOutlinedIcon}
tabName={TAB_NAME.SETTINGS}
onTabButtonClick={handleTabButtonClick}/>
</TabList>
<FileInfoTabPanel/>
<SearchTabPanel/>
<SettingsTabPanel/>
</Tabs>
);
});
};

SidebarTabs.displayName = "SidebarTabs";
export default SidebarTabs;
Loading