Skip to content

Commit

Permalink
Bugfix: Navigation with react-router-dom (#48)
Browse files Browse the repository at this point in the history
* Navigation bugfixes

* Do not set homepage in Header

Co-authored-by: Krithika Sundararajan <krithika.sundararajan@go-jek.com>
  • Loading branch information
krithika369 and Krithika Sundararajan committed Oct 26, 2022
1 parent 8c50881 commit 20df1c9
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 15 deletions.
16 changes: 11 additions & 5 deletions ui/src/AppRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@ import Home from "Home";

const App = () => {
const { appConfig } = useConfig();

return (
<Routes>
<Route path={appConfig.homepage} />
<Route index element={<Home />} />
<Route path="projects/:projectId">
<Route index element={<Navigate to="experiments" replace={true} />} />
<Route path="experiments/*" element={<ExperimentsLandingPage />} />
{/* We need this redirection because XP is not a recognized MLP app and so PrivateLayout
should not redirect to /xp directly on project change. */}
<Route path="projects/:projectId/*" element={<Navigate to={appConfig.homepage} replace={true} />} />
{/* ALL ROUTES */}
<Route path={appConfig.homepage}>
<Route index element={<Home />} />
<Route path="projects/:projectId">
<Route index element={<Navigate to="experiments" replace={true} />} />
<Route path="experiments/*" element={<ExperimentsLandingPage />} />
</Route>
</Route>
{/* DEFAULT */}
<Route path="*" element={<Navigate to="/pages/404" replace={true} />} />
Expand Down
11 changes: 7 additions & 4 deletions ui/src/Home.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { Fragment } from "react";

import React, { Fragment, useContext } from "react";
import { EuiPageTemplate } from "@elastic/eui";

import { ProjectsContext } from "@gojek/mlp-ui";
import { Navigate } from "react-router-dom";
import { useConfig } from "config";

const Home = () => {
const { appConfig } = useConfig();
const { currentProject } = useContext(ProjectsContext);

return (
return !currentProject ? (
<EuiPageTemplate>
<EuiPageTemplate.EmptyPrompt
iconType={appConfig.appIcon}
Expand All @@ -19,6 +20,8 @@ const Home = () => {
}
/>
</EuiPageTemplate>
) : (
<Navigate to={`projects/${currentProject.id}`} replace={true} />
);
};

Expand Down
8 changes: 4 additions & 4 deletions ui/src/PrivateLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ export const PrivateLayout = () => {
<ApplicationsContextProvider>
<ProjectsContextProvider>
<ApplicationsContext.Consumer>
{({ currentApp }) => (
{() => (
<Header
homepage={appConfig.homepage}
onProjectSelect={pId =>
navigate(urlJoin(currentApp?.href, "projects", pId, "experiments"))
navigate(urlJoin("projects", pId))
}
docLinks={appConfig.docsUrl}
/>)}
/>
)}
</ApplicationsContext.Consumer>
<Outlet />
</ProjectsContextProvider>
Expand Down
2 changes: 1 addition & 1 deletion ui/src/experiments/ExperimentsLandingPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const ExperimentsLandingPage = () => {
which results in incorrect /experiments/experiments prefix.
*/}
<Route
path="experiments"
path="experiments/*"
element={<Navigate to={location.pathname.replace("/experiments/experiments", "/experiments")}
replace={true} />} />
</Routes>
Expand Down
6 changes: 5 additions & 1 deletion ui/src/experiments/list/ListExperimentsTable.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from "react";

import { EuiHealth, EuiLink, EuiText } from "@elastic/eui";
import { useLocation } from "react-router-dom";
import urlJoin from "proper-url-join";

import { useConfig } from "config";
import { BasicTable } from "components/table/BasicTable";
Expand All @@ -17,6 +19,7 @@ const ListExperimentsTable = ({
onRowClick,
}) => {
const { appConfig } = useConfig();
const location = useLocation();
const tableColumns = [
{
field: "id",
Expand Down Expand Up @@ -88,11 +91,12 @@ const ListExperimentsTable = ({
width: "5%",
render: (item) => {
return (
// We need to do urlJoin to correctly handle trailing slash when used as a remote component
<EuiLink
onClick={(e) => {
e.stopPropagation();
}}
href={item.id}
href={urlJoin(location.pathname, item.id)}
target="_blank"
/>
);
Expand Down

0 comments on commit 20df1c9

Please sign in to comment.