Skip to content

Commit

Permalink
fix(web): do not render the overview page when selecting a product (#…
Browse files Browse the repository at this point in the history
…1331)

## Problem

When selecting a product, there is a small chance for the UI displaying
the MainLayout/OverviewPage while waiting for phase/status signals
changes. It happens because several reasons, among them the fact of a
few miss placed navigations/redirections.

## Solution

To add a new route for the product selection progress and relocate
navigation logic, mainly centralising it in the src/App.jsx
_intermediate router_.


## Testing

- Adapted existing tests
- Tested manually

---------

Co-authored-by: José Iván López González <jlopez@suse.com>
  • Loading branch information
dgdavid and joseivanlopez authored Jun 14, 2024
1 parent 9cdb5b9 commit 1153b17
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 56 deletions.
6 changes: 6 additions & 0 deletions web/package/agama-web-ui.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Fri Jun 14 13:55:27 UTC 2024 - David Diaz <dgonzalez@suse.com>

- Do not render the overview page when selecting a product
(gh#openSUSE/agama#1331).

-------------------------------------------------------------------
Fri Jun 14 07:37:58 UTC 2024 - David Diaz <dgonzalez@suse.com>

Expand Down
14 changes: 9 additions & 5 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
*/

import React, { useEffect, useState } from "react";
import { Navigate, Outlet, useLocation } from "react-router-dom";
import { Loading } from "./components/layout";
import { Outlet } from "react-router-dom";
import { ProductSelectionProgress } from "~/components/product";
import { Questions } from "~/components/questions";
import { ServerError, Installation } from "~/components/core";
import { useInstallerL10n } from "./context/installerL10n";
Expand All @@ -40,8 +39,9 @@ import { BUSY } from "~/client/status";
*/
function App() {
const client = useInstallerClient();
const location = useLocation();
const { connected, error } = useInstallerClientStatus();
const { products } = useProduct();
const { selectedProduct, products } = useProduct();
const { language } = useInstallerL10n();
const [status, setStatus] = useState(undefined);
const [phase, setPhase] = useState(undefined);
Expand Down Expand Up @@ -84,8 +84,12 @@ function App() {
return <Loading />;
}

if (phase === CONFIG && status === BUSY) {
return <ProductSelectionProgress />;
if (selectedProduct === null && location.pathname !== "/products") {
return <Navigate to="/products" />;
}

if (phase === CONFIG && status === BUSY && location.pathname !== "/products/progress") {
return <Navigate to="/products/progress" />;
}

return <Outlet />;
Expand Down
9 changes: 6 additions & 3 deletions web/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ jest.mock("~/client");

// list of available products
let mockProducts;
let mockSelectedProduct;

jest.mock("~/context/product", () => ({
...jest.requireActual("~/context/product"),
useProduct: () => {
return {
products: mockProducts,
selectedProduct: null
selectedProduct: mockSelectedProduct
};
}
}));
Expand Down Expand Up @@ -158,9 +160,9 @@ describe("App", () => {
getStatusFn.mockResolvedValue(BUSY);
});

it("renders the product selection progress", async () => {
it("redirects to product selection progress", async () => {
installerRender(<App />, { withL10n: true });
await screen.findByText(/Product progress/);
await screen.findByText("Navigating to /products/progress");
});
});

Expand All @@ -179,6 +181,7 @@ describe("App", () => {
describe("on the INSTALL phase", () => {
beforeEach(() => {
getPhaseFn.mockResolvedValue(INSTALL);
mockSelectedProduct = { id: "Fake product" };
});

it("renders the application content", async () => {
Expand Down
8 changes: 1 addition & 7 deletions web/src/components/overview/OverviewPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ import {
NotificationDrawerListItemHeader,
Stack,
} from "@patternfly/react-core";
import { useProduct } from "~/context/product";
import { useInstallerClient } from "~/context/installer";
import { Link, Navigate } from "react-router-dom";
import { Link } from "react-router-dom";
import { Center } from "~/components/layout";
import { CardField, EmptyState, Page, InstallButton } from "~/components/core";
import L10nSection from "./L10nSection";
Expand Down Expand Up @@ -88,18 +87,13 @@ const IssuesList = ({ issues }) => {
};

export default function OverviewPage() {
const { selectedProduct } = useProduct();
const [issues, setIssues] = useState([]);
const client = useInstallerClient();

useEffect(() => {
client.issues().then(setIssues);
}, [client]);

if (selectedProduct === null) {
return <Navigate to="/products" />;
}

const resultSectionProps =
issues.isEmpty
? {}
Expand Down
11 changes: 0 additions & 11 deletions web/src/components/overview/OverviewPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,6 @@ beforeEach(() => {
});
});

describe("when no product is selected", () => {
beforeEach(() => {
mockSelectedProduct = null;
});

it("redirects to the products page", async () => {
installerRender(<OverviewPage />);
screen.getByText("Navigating to /products");
});
});

describe("when a product is selected", () => {
beforeEach(() => {
mockSelectedProduct = { name: "Tumbleweed" };
Expand Down
9 changes: 4 additions & 5 deletions web/src/components/product/ProductSelectionPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
*/

import React, { useState } from "react";
import { useNavigate } from "react-router-dom";
import {
Card, CardBody,
Flex,
Expand All @@ -42,18 +41,17 @@ const Label = ({ children }) => (
);

function ProductSelectionPage() {
const navigate = useNavigate();
const { products, selectedProduct, selectProduct } = useProduct();
const [nextProduct, setNextProduct] = useState(selectedProduct);
const [isLoading, setIsLoading] = useState(false);

const onSubmit = async (e) => {
e.preventDefault();

if (nextProduct) {
await selectProduct(nextProduct.id);
setIsLoading(true);
}

navigate("/");
};

if (!products) return (
Expand Down Expand Up @@ -93,11 +91,12 @@ function ProductSelectionPage() {
))}
<Item>
<Flex justifyContent={{ default: "justifyContentFlexEnd" }}>
{selectedProduct && <Page.CancelAction navigateTo={-1} />}
{selectedProduct && !isLoading && <Page.CancelAction navigateTo={-1} />}
<Page.Action
type="submit"
form="productSelectionForm"
isDisabled={isSelectionDisabled}
isLoading={isLoading}
>
{_("Select")}
</Page.Action>
Expand Down
56 changes: 32 additions & 24 deletions web/src/components/product/ProductSelectionProgress.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/

import React, { useEffect, useState } from "react";
import { Navigate } from "react-router-dom";
import {
Card, CardBody,
Grid, GridItem,
Expand All @@ -30,10 +31,10 @@ import {

import { _ } from "~/i18n";
import { Center } from "~/components/layout";
import SimpleLayout from "~/SimpleLayout";
import { useCancellablePromise } from "~/utils";
import { useInstallerClient } from "~/context/installer";
import { useProduct } from "~/context/product";
import { IDLE } from "~/client/status";

const Progress = ({ selectedProduct, storageProgress, softwareProgress }) => {
const variant = (progress) => {
Expand Down Expand Up @@ -107,10 +108,16 @@ const Progress = ({ selectedProduct, storageProgress, softwareProgress }) => {
*/
function ProductSelectionProgress() {
const { cancellablePromise } = useCancellablePromise();
const { storage, software } = useInstallerClient();
const { manager, storage, software } = useInstallerClient();
const { selectedProduct } = useProduct();
const [storageProgress, setStorageProgress] = useState({});
const [softwareProgress, setSoftwareProgress] = useState({});
const [status, setStatus] = useState();

useEffect(() => {
manager.getStatus().then(setStatus);
return manager.onStatusChange(setStatus);
}, [manager, setStatus]);

useEffect(() => {
const updateProgress = (progress) => {
Expand All @@ -136,29 +143,30 @@ function ProductSelectionProgress() {
return software.onProgressChange(updateProgress);
}, [cancellablePromise, setSoftwareProgress, software]);

if (status === IDLE) return <Navigate to="/" replace />;

return (
<SimpleLayout showOutlet={false}>
<Center>
<Grid hasGutter>
<GridItem sm={8} smOffset={2}>
<Card isPlain>
<CardBody>
<Stack hasGutter>
<h1 style={{ textAlign: "center" }}>
{_("Configuring the product, please wait ...")}
</h1>
<Progress
selectedProduct={selectedProduct}
storageProgress={storageProgress}
softwareProgress={softwareProgress}
/>
</Stack>
</CardBody>
</Card>
</GridItem>
</Grid>
</Center>
</SimpleLayout>
<Center>
<Grid hasGutter>
<GridItem sm={8} smOffset={2}>
<Card isPlain>
<CardBody>
<Stack hasGutter>
<h1 style={{ textAlign: "center" }}>
{_("Configuring the product, please wait ...")}
</h1>
{ status &&
<Progress
selectedProduct={selectedProduct}
storageProgress={storageProgress}
softwareProgress={softwareProgress}
/>}
</Stack>
</CardBody>
</Card>
</GridItem>
</Grid>
</Center>
);
}

Expand Down
14 changes: 13 additions & 1 deletion web/src/components/product/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,23 @@
*/

import React from "react";
import { Page } from "~/components/core";
import ProductSelectionPage from "./ProductSelectionPage";
import ProductSelectionProgress from "./ProductSelectionProgress";

const productsRoute = {
path: "/products",
element: <ProductSelectionPage />
element: <Page />,
children: [
{
index: true,
element: <ProductSelectionPage />
},
{
path: "progress",
element: <ProductSelectionProgress />
}
]
};

export {
Expand Down

0 comments on commit 1153b17

Please sign in to comment.