From 19cc6893ab435c825be6ae25ac1fbc0b21c15278 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 1 May 2024 04:26:48 -0400 Subject: [PATCH] Bye tabs, hi URLs (#389) --- src/App.tsx | 18 +-- .../components/EnvironmentCreate.tsx | 40 +++--- .../components/EnvironmentDetails.tsx | 116 +++++++++++++----- .../environments/components/Environment.tsx | 6 +- .../components/EnvironmentDropdown.tsx | 53 ++------ .../environments/components/Environments.tsx | 78 ++++++------ src/features/environments/reducer.ts | 60 +++++---- src/features/namespaces/reducer.ts | 30 +++-- src/features/notification/index.ts | 1 + .../notification/notificationSlice.ts | 30 +++++ .../components/AddRequestedPackage.tsx | 1 - src/features/tabs/components/PageTabs.tsx | 114 ----------------- src/features/tabs/components/index.tsx | 1 - src/features/tabs/index.tsx | 1 - src/features/tabs/tabsSlice.ts | 8 +- src/layouts/PageLayout.tsx | 70 ++++------- src/{AppExample.tsx => main.tsx} | 0 src/routes.tsx | 26 ++++ src/store/rootReducer.ts | 10 +- src/styles/StyledIconButton.tsx | 2 +- src/styles/StyledTab.tsx | 23 ---- src/styles/StyledTabs.tsx | 13 -- src/styles/index.tsx | 2 - src/utils/helpers/namespaces.ts | 16 ++- test/PageTabs.test.tsx | 93 -------------- .../EnvironmentCreate.test.tsx | 9 +- .../EnvironmentDetails.test.tsx | 17 +-- .../EnvironmentDetailsHeader.test.tsx | 18 ++- .../SpecificationEdit.test.tsx | 13 +- test/environments/Environment.test.tsx | 20 +-- .../environments/EnvironmentDropdown.test.tsx | 20 +-- test/environments/EnvironmentList.test.tsx | 7 +- test/environments/Environments.test.tsx | 9 +- test/playwright/test_ux.py | 43 +++---- .../AddRequestedPackage.test.tsx | 17 +-- webpack.config.js | 7 +- 36 files changed, 403 insertions(+), 589 deletions(-) create mode 100644 src/features/notification/index.ts create mode 100644 src/features/notification/notificationSlice.ts delete mode 100644 src/features/tabs/components/PageTabs.tsx delete mode 100644 src/features/tabs/components/index.tsx rename src/{AppExample.tsx => main.tsx} (100%) create mode 100644 src/routes.tsx delete mode 100644 src/styles/StyledTab.tsx delete mode 100644 src/styles/StyledTabs.tsx delete mode 100644 test/PageTabs.test.tsx diff --git a/src/App.tsx b/src/App.tsx index dae49ed7..f469c10e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,32 +1,20 @@ import { ThemeProvider } from "@mui/material"; import React from "react"; import { Provider } from "react-redux"; -import { BrowserRouter as Router } from "react-router-dom"; -import { Route, Routes } from "react-router"; +import { RouterProvider } from "react-router-dom"; -import { PageLayout } from "./layouts"; import { IPreferences, PrefContext, prefDefault, prefGlobal } from "./preferences"; +import { router } from "./routes"; import { store } from "./store"; import { condaStoreTheme, grayscaleTheme } from "./theme"; import "../style/index.css"; -const AppRouter = () => { - // for now, trivial routing is sufficient - return ( - - - } /> - - - ); -}; - export interface IAppProps { pref?: Partial; } @@ -86,7 +74,7 @@ export class App< } > - + diff --git a/src/features/environmentCreate/components/EnvironmentCreate.tsx b/src/features/environmentCreate/components/EnvironmentCreate.tsx index f369202c..a23306fb 100644 --- a/src/features/environmentCreate/components/EnvironmentCreate.tsx +++ b/src/features/environmentCreate/components/EnvironmentCreate.tsx @@ -1,4 +1,5 @@ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; +import { useNavigate, useParams } from "react-router-dom"; import Box from "@mui/material/Box"; import Alert from "@mui/material/Alert"; import { stringify } from "yaml"; @@ -16,23 +17,38 @@ import { } from "../../../features/metadata"; import { environmentOpened, - closeCreateNewEnvironmentTab + closeCreateNewEnvironmentTab, + openCreateNewEnvironmentTab } from "../../../features/tabs"; import { useAppDispatch, useAppSelector } from "../../../hooks"; import { SpecificationCreate, SpecificationReadOnly } from "./Specification"; import { descriptionChanged, nameChanged } from "../environmentCreateSlice"; import createLabel from "../../../common/config/labels"; - -export interface IEnvCreate { - environmentNotification: (notification: any) => void; -} +import { showNotification } from "../../notification/notificationSlice"; interface ICreateEnvironmentArgs { code: { channels: string[]; dependencies: string[] }; } -export const EnvironmentCreate = ({ environmentNotification }: IEnvCreate) => { +export const EnvironmentCreate = () => { const dispatch = useAppDispatch(); + + // Url routing params + // If user loads the app at //new-environment + // This will put the app in the correct state + const { namespaceName } = useParams<{ + namespaceName: string; + }>(); + + useEffect(() => { + if (namespaceName) { + dispatch(modeChanged(EnvironmentDetailsModes.CREATE)); + dispatch(openCreateNewEnvironmentTab(namespaceName)); + } + }, [namespaceName]); + + const navigate = useNavigate(); + const { mode } = useAppSelector(state => state.environmentDetails); const { name, description } = useAppSelector( state => state.environmentCreate @@ -84,17 +100,13 @@ export const EnvironmentCreate = ({ environmentNotification }: IEnvCreate) => { dispatch( environmentOpened({ environment, - selectedEnvironmentId: newEnvId, canUpdate: true }) ); + // After new environment has been created, navigate to the new environment's web page + navigate(`/${namespace}/${name}`); dispatch(currentBuildIdChanged(data.build_id)); - environmentNotification({ - data: { - show: true, - description: createLabel(name, "create") - } - }); + dispatch(showNotification(createLabel(name, "create"))); } catch (e) { setError({ message: e?.data?.message ?? createLabel(undefined, "error"), diff --git a/src/features/environmentDetails/components/EnvironmentDetails.tsx b/src/features/environmentDetails/components/EnvironmentDetails.tsx index 7b8e690e..21d1ed37 100644 --- a/src/features/environmentDetails/components/EnvironmentDetails.tsx +++ b/src/features/environmentDetails/components/EnvironmentDetails.tsx @@ -1,4 +1,6 @@ import React, { useEffect, useState } from "react"; +import { skipToken } from "@reduxjs/toolkit/query/react"; +import { useNavigate, useParams } from "react-router-dom"; import Box from "@mui/material/Box"; import Alert from "@mui/material/Alert"; import { stringify } from "yaml"; @@ -8,7 +10,9 @@ import { SpecificationEdit, SpecificationReadOnly } from "./Specification"; import { useGetBuildQuery } from "../environmentDetailsApiSlice"; import { updateEnvironmentBuildId, - environmentClosed + environmentClosed, + environmentOpened, + toggleNewEnvironmentView } from "../../../features/tabs"; import { useGetBuildPackagesQuery, @@ -34,13 +38,15 @@ import artifactList from "../../../utils/helpers/artifact"; import createLabel from "../../../common/config/labels"; import { AlertDialog } from "../../../components/Dialog"; import { useAppDispatch, useAppSelector } from "../../../hooks"; -import { CondaSpecificationPip } from "../../../common/models"; +import { + CondaSpecificationPip, + Environment, + Namespace +} from "../../../common/models"; import { useInterval } from "../../../utils/helpers"; +import { showNotification } from "../../notification/notificationSlice"; +import { useScrollRef } from "../../../layouts/PageLayout"; -interface IEnvDetails { - scrollRef: any; - environmentNotification: (notification: any) => void; -} interface IUpdateEnvironmentArgs { dependencies: (string | CondaSpecificationPip)[]; channels: string[]; @@ -48,16 +54,65 @@ interface IUpdateEnvironmentArgs { const INTERVAL_REFRESHING = 5000; -export const EnvironmentDetails = ({ - scrollRef, - environmentNotification -}: IEnvDetails) => { +export const EnvironmentDetails = () => { const dispatch = useAppDispatch(); + + // Url routing params + // If user loads the app at // + // This will put the app in the correct state + const { namespaceName, environmentName } = useParams<{ + namespaceName: string; + environmentName: string; + }>(); + const namespaces: Namespace[] = useAppSelector( + state => state.namespaces.data + ); + const namespace = namespaces.find(({ name }) => name === namespaceName); + const environments: Environment[] = useAppSelector( + state => state.environments.data + ); + const environment = environments.find( + environment => + environment.namespace.name === namespaceName && + environment.name === environmentName + ); + useEffect(() => { + if (namespace && environment) { + dispatch( + environmentOpened({ + environment, + canUpdate: namespace.canUpdate + }) + ); + dispatch(modeChanged(EnvironmentDetailsModes.READ)); + dispatch(toggleNewEnvironmentView(false)); + } + }, [ + // We only want to run this effect when: + // + // 1. User navigates to different environment = change of + // (namespaceName, environmentName) in the URL + // 2. The corresponding (namespace, environment) data have been fetched + // + // We cannot pass [namespace, environment] as the dependencies to + // useEffect() because whenever an environment is created or updated, a + // refetch of the data is triggered, which creates new data objects for + // [namespace, environment] in the Redux store, which would cause this + // effect to rerun, but, again, we only want to run this effect when the + // user navigates to a new (namespaceName, environmentName), hence the `&& + // namespace.name` and `&& environment.name`. + namespace && namespace.name, + environment && environment.name + ]); + + const navigate = useNavigate(); + const { mode } = useAppSelector(state => state.environmentDetails); const { page, dependencies } = useAppSelector(state => state.dependencies); const { selectedEnvironment } = useAppSelector(state => state.tabs); const { currentBuild } = useAppSelector(state => state.enviroments); const [name, setName] = useState(selectedEnvironment?.name || ""); + const scrollRef = useScrollRef(); const [descriptionIsUpdated, setDescriptionIsUpdated] = useState(false); const [description, setDescription] = useState( @@ -81,7 +136,7 @@ export const EnvironmentDetails = ({ const [updateBuildId] = useUpdateBuildIdMutation(); const [deleteEnvironment] = useDeleteEnvironmentMutation(); - useGetEnviromentBuildsQuery(selectedEnvironment, { + useGetEnviromentBuildsQuery(selectedEnvironment ?? skipToken, { pollingInterval: INTERVAL_REFRESHING }); @@ -112,7 +167,7 @@ export const EnvironmentDetails = ({ }; const loadArtifacts = async () => { - if (artifactType.includes("DOCKER_MANIFEST")) { + if (!currentBuildId || artifactType.includes("DOCKER_MANIFEST")) { return; } @@ -122,7 +177,7 @@ export const EnvironmentDetails = ({ }; const loadDependencies = async () => { - if (dependencies.length) { + if (!currentBuildId || dependencies.length) { return; } @@ -171,12 +226,7 @@ export const EnvironmentDetails = ({ dispatch(modeChanged(EnvironmentDetailsModes.READ)); setCurrentBuildId(data.build_id); dispatch(currentBuildIdChanged(data.build_id)); - environmentNotification({ - data: { - show: true, - description: createLabel(environment, "update") - } - }); + dispatch(showNotification(createLabel(environment, "update"))); } catch (e) { setError({ message: @@ -187,7 +237,7 @@ export const EnvironmentDetails = ({ visible: true }); } - scrollRef.current.scrollTo(0, 0); + scrollRef.current?.scrollTo(0, 0); }; const updateBuild = async (buildId: number) => { @@ -201,12 +251,9 @@ export const EnvironmentDetails = ({ buildId }).unwrap(); dispatch(updateEnvironmentBuildId(buildId)); - environmentNotification({ - data: { - show: true, - description: createLabel(selectedEnvironment.name, "updateBuild") - } - }); + dispatch( + showNotification(createLabel(selectedEnvironment.name, "updateBuild")) + ); } catch (e) { setError({ message: createLabel(undefined, "error"), @@ -232,19 +279,17 @@ export const EnvironmentDetails = ({ selectedEnvironmentId: selectedEnvironment.id }) ); - environmentNotification({ - data: { - show: true, - description: createLabel(selectedEnvironment.name, "delete") - } - }); + dispatch( + showNotification(createLabel(selectedEnvironment.name, "delete")) + ); + navigate("/"); } catch (e) { setError({ message: createLabel(undefined, "error"), visible: true }); } - scrollRef.current.scrollTo(0, 0); + scrollRef.current?.scrollTo(0, 0); setShowDialog(false); }; @@ -255,10 +300,15 @@ export const EnvironmentDetails = ({ })(); }, INTERVAL_REFRESHING); + if (!selectedEnvironment) { + return null; + } + return ( diff --git a/src/features/environments/components/Environment.tsx b/src/features/environments/components/Environment.tsx index 1f35642b..31dfc65d 100644 --- a/src/features/environments/components/Environment.tsx +++ b/src/features/environments/components/Environment.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { Link } from "react-router-dom"; import CircleIcon from "@mui/icons-material/Circle"; import ListItemIcon from "@mui/material/ListItemIcon"; import Button from "@mui/material/Button"; @@ -12,13 +13,11 @@ interface IEnvironmentProps { * @param selectedEnvironmentId id of the currently selected environment */ environment: EnvironmentModel; - onClick: () => void; selectedEnvironmentId: number | undefined; } export const Environment = ({ environment, - onClick, selectedEnvironmentId }: IEnvironmentProps) => { const isSelected = selectedEnvironmentId === environment.id; @@ -44,7 +43,8 @@ export const Environment = ({ />