Skip to content

feat(new-webui): Add support for page switching using react-router. #797

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

Merged
davemarco merged 22 commits into from
Apr 9, 2025
Merged

feat(new-webui): Add support for page switching using react-router. #797

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e7dce76
draft
Mar 30, 2025
df9c602
fix lint
Mar 30, 2025
3c475f6
add files
Mar 30, 2025
e554796
small nits
Mar 30, 2025
976c889
small
Mar 30, 2025
1587060
set to false
Mar 30, 2025
2f3582c
small
Mar 31, 2025
2837d59
junhao review
Apr 1, 2025
e4a4be6
small change
Apr 1, 2025
25ce98a
Fix lint
Apr 1, 2025
712af39
router
Apr 1, 2025
f433ee7
merge main
Apr 4, 2025
d84bcf9
modules.css
Apr 4, 2025
02bbfc7
fix lint
Apr 4, 2025
c36902c
lastest
Apr 4, 2025
6acc4f9
Apply suggestions from code review
davemarco Apr 7, 2025
240e4b8
latest
Apr 8, 2025
4049669
merge
Apr 8, 2025
2a9c75e
review
Apr 8, 2025
f8f176c
lint
Apr 8, 2025
4d40764
latest:
Apr 8, 2025
748b3c5
review
Apr 9, 2025
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
54 changes: 53 additions & 1 deletion components/log-viewer-webui/client/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion components/log-viewer-webui/client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"antd": "^5.24.5",
"axios": "^1.7.9",
"react": "^19.0.0",
"react-dom": "^19.0.0"
"react-dom": "^19.0.0",
"react-router": "^7.4.1"
},
"devDependencies": {
"@types/react": "^19.0.10",
Expand Down
8 changes: 4 additions & 4 deletions components/log-viewer-webui/client/src/AntdApp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import MainLayout from "./ui/MainLayout";
import {RouterProvider} from "react-router";

import router from "./routes";


/**
Expand All @@ -7,9 +9,7 @@ import MainLayout from "./ui/MainLayout";
* @return
*/
const AntApp = () => {
return (
<MainLayout/>
);
return <RouterProvider router={router}/>;
};

export default AntApp;
6 changes: 3 additions & 3 deletions ...viewer-webui/client/src/ui/MainLayout.css → ...c/components/Layout/MainLayout.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
.main-layout {
.mainLayout {
min-height: 100vh;
}

.sider-logo-container {
.siderLogoContainer {
display: flex;
justify-content: center;
align-items: center;
height: 64px;
}

.sider-logo {
.siderLogo {
max-width: 100%;
max-height: 100%;
height: 32px;
Expand Down
23 changes: 15 additions & 8 deletions ...viewer-webui/client/src/ui/MainLayout.tsx → ...ient/src/components/Layout/MainLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import {useState} from "react";
import {
Link,
Outlet,
} from "react-router";
Comment thread
davemarco marked this conversation as resolved.
Resolved

import {
SearchOutlined,
Expand All @@ -10,16 +14,16 @@ import {
MenuProps,
} from "antd";

import "./MainLayout.css";
import styles from "./MainLayout.module.css";


const {Sider} = Layout;

type MenuItem = Required<MenuProps>["items"][number];

const items: MenuItem[] = [
{label: "Ingest", key: "/ingest", icon: <UploadOutlined/>},
{label: "Search", key: "/search", icon: <SearchOutlined/>},
const SIDEBAR_MENU_ITEMS: MenuItem[] = [
{label: <Link to={"/ingest"}>Ingest</Link>, key: "/ingest", icon: <UploadOutlined/>},
{label: <Link to={"/search"}>Search</Link>, key: "/search", icon: <SearchOutlined/>},
];

/**
Expand All @@ -31,7 +35,7 @@ const MainLayout = () => {
const [collapsed, setCollapsed] = useState(false);

return (
<Layout className={"main-layout"}>
<Layout className={styles.mainLayout}>
<Sider
collapsed={collapsed}
collapsible={true}
Expand All @@ -41,16 +45,19 @@ const MainLayout = () => {
setCollapsed(value);
}}
>
<div className={"sider-logo-container"}>
<div className={styles.siderLogoContainer}>
<img
alt={"CLP Logo"}
className={"sider-logo"}
className={styles.siderLogo}
src={"/clp-logo.png"}/>
</div>
<Menu
items={items}
items={SIDEBAR_MENU_ITEMS}
mode={"inline"}/>
</Sider>
<Layout>
<Outlet/>
</Layout>
</Layout>
);
};
Expand Down
16 changes: 16 additions & 0 deletions components/log-viewer-webui/client/src/pages/IngestPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Presents compression statistics.
*
* @return
*/
const IngestView = () => {
return (
<div>
<h1>Ingest View</h1>
<p>This is the Ingest view.</p>
</div>
);
};


export default IngestView;
16 changes: 16 additions & 0 deletions components/log-viewer-webui/client/src/pages/SearchPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Provides a search interface that allows users to query archives and visualize search results.
*
* @return
*/
const SearchView = () => {
return (
<div>
<h1>Search View</h1>
<p>This is the Search view.</p>
</div>
);
};


export default SearchView;
20 changes: 20 additions & 0 deletions components/log-viewer-webui/client/src/routes.tsx
View file
Open in desktop
Comment thread
davemarco marked this conversation as resolved.
Resolved

@junhaoliao junhaoliao Apr 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. don't forget updating the page component names inside the files.
  2. since this routes.tsx default exports router, let's name this file router.tsx to match the default export name then.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {createBrowserRouter} from "react-router";

import MainLayout from "./components/Layout/MainLayout";
import IngestView from "./pages/IngestPage";
import SearchView from "./pages/SearchPage";


const router = createBrowserRouter([

@junhaoliao junhaoliao Apr 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const router = createBrowserRouter([
const ROUTER = createBrowserRouter([

@davemarco davemarco Apr 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I took this syntax from their docs. Unless u have strong opinions i would leave it to match

@junhaoliao junhaoliao Apr 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we've been using SNAKE_CASE for global constants but i do get variables are mostly const since EM6. I prefer keeping SNAKE_CASES, but let's quickly check with @kirkrodrigues before we commit.

@davemarco davemarco Apr 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Router may also make sense if it is a component

@junhaoliao junhaoliao Apr 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I discussed this offline with @kirkrodrigues and here're some consolidated thoughts (a copy of section https://www.notion.so/yscope/WIP-Coding-Guidelines-9a308b847a5343958ba3cb97a850be66?pvs=4#1cf04e4d9e6b80c7ac4fe69b846f5791)

Naming convention for constants in JavaScript / TypeScript

JavaScript doesn't enforce deep immutability for const objects or arrays, so we apply a semantic naming convention to indicate intent:

  • SCREAMING_SNAKE_CASE

    Use this for truly immutable values. This includes:

    • Primitive constant values
      e.g., const SECONDS_IN_MINUTE = 60;
    • Deeply frozen objects
      e.g., const CONFIG = Object.freeze({...});
    • Any value that is intended to remain constant throughout its lifetime.

  • camelCase

    Use this for variables declared with const whose contents may still change (e.g., properties of objects or elements of arrays). This keeps the code readable and avoids excessive SCREAMING_SNAKE_CASE.

Additional Notes

  • React useState variables should not use SCREAMING_SNAKE_CASE, even though the bindings are immutable. Their values can change via re-renders, so they are not considered truly constant.

  • Constant functions should follow normal function naming rules, based on their role:

    • Use PascalCase for JSX functional components (e.g., const MyComponent = () => { ... }), as required by React.
    • Use camelCase for all other functions (e.g., const formatDate = () => { ... }).

    Even though function bindings with const are immutable, the function itself is a callable entity, not a constant value, so we do not use SCREAMING_SNAKE_CASE.

@junhaoliao junhaoliao Apr 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That said, let's make this object deeply frozen since the routes are not expected to be modified elsewhere. We will then rename the object and the filename to ROUTES as well.

@davemarco davemarco Apr 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this router has state and manages stuff like breadcrumbs. See their comment.
Screenshot 2025-04-08 at 6 07 18 PM

I dont think its just a config. Anyways for now as router... i left it. Let me know if u disagree

@junhaoliao junhaoliao Apr 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sure. i just thought of another reason that we don't want to make it deeply frozen: if we are to support any plugins like an optional page. this is fine to leave then

{
path: "/",
Component: MainLayout,
children: [
{path: "ingest", Component: IngestView},
{path: "search", Component: SearchView},
],
},
]);

Comment thread
davemarco marked this conversation as resolved.
Resolved

export default router;