Skip to content

Commit

Permalink
fix(web): reduce ProgressReport flickering (#1395)
Browse files Browse the repository at this point in the history
## Problem

ProgressReport component is using the PF/ProgressStepper in a dynamic
way, which produces quite a few annoying UI flickering.

See
#1373 (comment)


## Solution

To mitigate these flickering by forcing a fixed _inline-size_ for each
step and making use of the PatternFly/Truncate component.

A final solution needs more time to think about the whole component.
  • Loading branch information
dgdavid authored Jun 26, 2024
1 parent 218adbf commit c56bd4a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 17 deletions.
5 changes: 5 additions & 0 deletions web/package/agama-web-ui.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
-------------------------------------------------------------------
Wed Jun 26 16:46:58 UTC 2024 - David Diaz <dgonzalez@suse.com>

- Reduce progress report flickering (gh#openSUSE/agama#1395).

-------------------------------------------------------------------
Wed Jun 26 15:57:52 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

Expand Down
6 changes: 2 additions & 4 deletions web/src/assets/styles/patternfly-overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,8 @@ table td > .pf-v5-c-empty-state {
}

.pf-v5-c-progress-stepper.progress-report {
.pf-v5-c-progress-stepper__step-connector,
.pf-v5-c-progress-stepper__step-main {
inline-size: 220px;
display: flex;
flex-direction: column;
row-gap: 1em;
inline-size: 250px;
}
}
24 changes: 13 additions & 11 deletions web/src/components/core/ProgressReport.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import React, { useEffect, useState } from "react";
import {
Card,
CardBody,
Flex,
Grid,
GridItem,
ProgressStepper,
ProgressStep,
ProgressStepper,
Spinner,
Text,
TextVariants,
Flex,
Stack,
Truncate
} from "@patternfly/react-core";

import { _ } from "~/i18n";
Expand All @@ -47,25 +47,27 @@ const Progress = ({ steps, step, firstStep, detail }) => {

if (stepNumber < step.current) {
properties.variant = "success";
properties.description = <Text component={TextVariants.p}>{_("Finished")}</Text>;
properties.description = <div>{_("Finished")}</div>;
}

if (properties.isCurrent) {
properties.variant = "info";
if (detail && detail.message !== "") {
const { message, current, total } = detail;
properties.description = (
<>
<Text component={TextVariants.p}>{_("In progress")}</Text>
<Text component={TextVariants.p}>{`${message} (${current}/${total})`}</Text>
</>
<Stack hasGutter>
<div>{_("In progress")}</div>
<div>
<Truncate content={`${message} (${current}/${total})`} trailingNumChars={12} position="middle" />
</div>
</Stack>
);
}
}

if (stepNumber > step.current) {
properties.variant = "pending";
properties.description = <Text component={TextVariants.p}>{_("Pending")}</Text>;
properties.description = <div>{_("Pending")}</div>;
}

return properties;
Expand Down Expand Up @@ -133,7 +135,7 @@ function ProgressReport({ title, firstStep }) {
return (
<Center>
<Grid hasGutter>
<GridItem sm={8} smOffset={2}>
<GridItem sm={10} smOffset={1}>
<Card isPlain>
<CardBody>
<Flex direction={{ default: "column" }} rowGap={{ default: "rowGap2xl" }} alignItems={{ default: "alignItemsCenter" }}>
Expand Down
9 changes: 7 additions & 2 deletions web/src/components/core/ProgressReport.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ describe("ProgressReport", () => {
});
});

await screen.findByText("Doing some partitioning (1/10)");
// NOTE: not finding the whole text because it is now split in two <span> because of PF/Truncate
await screen.findByText(/Doing some/);
await screen.findByText(/\(1\/10\)/);
});

it("shows the progress including the details from the software service", async () => {
Expand All @@ -108,7 +110,10 @@ describe("ProgressReport", () => {
});
});

await screen.findByText("Installing packages (495/500)");
// NOTE: not finding the whole "Intalling packages (495/500)" because it
// is now split in two <span> because of PF/Truncate
await screen.findByText(/Installing/);
await screen.findByText(/.*\(495\/500\)/);
});
});
});

0 comments on commit c56bd4a

Please sign in to comment.