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.

Sign up for GitHub

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

docs: Add link to docs in settings dialog and sidebar. #134

Merged
merged 7 commits into from
Nov 28, 2024
Merged

docs: Add link to docs in settings dialog and sidebar. #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ede2ac
start
davemarco Nov 28, 2024
2856d1b
more
davemarco Nov 28, 2024
d3a0707
fix weird revert
davemarco Nov 28, 2024
1fc7f32
Apply suggestions from code review
davemarco Nov 28, 2024
acee3c3
kirk review
davemarco Nov 28, 2024
53491e7
kirk review
davemarco Nov 28, 2024
98bad59
junhao review
davemarco Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/components/CentralContainer/Sidebar/SidebarTabs/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import {
} from "@mui/joy";
import SvgIcon from "@mui/material/SvgIcon";

import HelpOutlineIcon from "@mui/icons-material/HelpOutline";
import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined";
import SearchIcon from "@mui/icons-material/Search";
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";
Expand All @@ -23,6 +25,8 @@ import TabButton from "./TabButton";
import "./index.css";


const DOCUMENTATION_URL = "https://docs.yscope.com/yscope-log-viewer/main/user-guide/index.html";
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Documentation URL is not accessible and returns a 404 error

The hardcoded documentation URL https://docs.yscope.com/yscope-log-viewer/main/user-guide/index.html is not accessible and returns a 404 error. This needs to be addressed before deployment.

  • Consider verifying the correct documentation URL with the team
  • Move the URL to a configuration file for easier maintenance
  • Ensure the documentation is published and accessible before deploying this feature
🔗 Analysis chain

Consider making the documentation URL configurable

The hardcoded URL points to a specific version ("main") and might need to be updated when docs structure changes or different versions are supported.

Consider moving this to a configuration file or environment variable for easier maintenance.

Let's verify if the documentation URL is accessible:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the documentation URL is accessible
curl -I https://docs.yscope.com/yscope-log-viewer/main/user-guide/index.html

Length of output: 1281


/**
* Lists information for each tab.
*/
Expand Down Expand Up @@ -63,6 +67,9 @@ const SidebarTabs = forwardRef<HTMLDivElement, SidebarTabsProps>((
case TAB_NAME.SETTINGS:
setIsSettingsModalOpen(true);
break;
case TAB_NAME.DOCUMENTATION:
openInNewTab(DOCUMENTATION_URL);
break;
default:
onActiveTabNameChange(tabName);
}
Expand All @@ -88,9 +95,14 @@ const SidebarTabs = forwardRef<HTMLDivElement, SidebarTabsProps>((
onTabButtonClick={handleTabButtonClick}/>
))}

{/* Forces the settings tab to bottom of sidebar. */}
{/* 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}
Expand Down
24 changes: 17 additions & 7 deletions src/components/modals/SettingsModal/SettingsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
FormHelperText,
FormLabel,
Input,
Link,
ModalDialog,
} from "@mui/joy";

Expand All @@ -33,13 +34,22 @@ import ThemeSwitchToggle from "./ThemeSwitchToggle";

const CONFIG_FORM_FIELDS = [
{
helperText: `[JSON] Log message conversion pattern: use field placeholders to insert
values from JSON log events. The syntax is
\`{<field-name>[:<formatter-name>[:<formatter-options>]]}\`, where \`field-name\` is
required, while \`formatter-name\` and \`formatter-options\` are optional. For example,
the following placeholder would format a timestamp field with name \`@timestamp\`:
\`{@timestamp:timestamp:YYYY-MM-DD HH\\:mm\\:ss.SSS}\`. Leave format string blank to
display the entire log event formatted as JSON.`,
helperText: (
<p>
[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.
</p>
),
initialValue: getConfig(CONFIG_KEY.DECODER_OPTIONS).formatString,
label: "Decoder: Format string",
name: LOCAL_STORAGE_KEY.DECODER_OPTIONS_FORMAT_STRING,
Expand Down
2 changes: 2 additions & 0 deletions src/typings/tab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
enum TAB_NAME {
NONE = "none",
DOCUMENTATION = "documentation",
FILE_INFO = "fileInfo",
SEARCH = "search",
SETTINGS = "settings",
Expand All @@ -10,6 +11,7 @@ enum TAB_NAME {
*/
const TAB_DISPLAY_NAMES: Record<TAB_NAME, string> = Object.freeze({
[TAB_NAME.NONE]: "None",
[TAB_NAME.DOCUMENTATION]: "Documentation",
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Documentation tab should handle unavailable content gracefully

The current implementation directly opens the documentation URL in a new tab without any error handling. If the documentation is not yet available, this could lead to a poor user experience when users encounter a 404 error or unavailable page.

  • src/components/CentralContainer/Sidebar/SidebarTabs/index.tsx: Add error handling or disable the documentation tab until the content is ready
  • Consider one of these approaches:
    • Disable the tab and show a tooltip indicating "Documentation coming soon"
    • Add a modal/notification when clicked to inform users about the documentation status
    • Remove the tab entirely until documentation is available
🔗 Analysis chain

Verify handling of unavailable documentation

Since the documentation is currently unavailable (as mentioned in the PR description), please ensure appropriate error handling or user feedback is implemented when users click this tab.

Let's check for any error handling in the click handler:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for error handling in documentation tab click handler
# Expected: Find implementation showing how unavailable documentation is handled

# Search for documentation URL constant and its usage
rg -A 5 "DOCUMENTATION_URL|handleTabButtonClick.*documentation" --type ts

Length of output: 1170

Script:

#!/bin/bash
# Let's verify if the documentation URL is actually accessible
# and if there's any conditional rendering or error handling around it

# Search for any conditional rendering related to documentation tab
rg -B 2 -A 5 "TAB_NAME\.DOCUMENTATION" --type ts

# Also check for any error handling patterns around documentation
rg -B 2 -A 5 "docs\.yscope\.com" --type ts

Length of output: 3567

[TAB_NAME.FILE_INFO]: "File info",
[TAB_NAME.SEARCH]: "Search",
[TAB_NAME.SETTINGS]: "Settings",
Expand Down
9 changes: 9 additions & 0 deletions src/utils/url.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,17 @@ const getBasenameFromUrlOrDefault = (
return basename;
};

/**
* Opens a given URL in a new browser tab.
*
* @param url
*/
const openInNewTab = (url: string): void => {
window.open(url, "_blank", "noopener");
};

export {
getAbsoluteUrl,
getBasenameFromUrlOrDefault,
openInNewTab,
};