Skip to content

Commit

Permalink
Avoid endless spinner when no readme is provided (#6590)
Browse files Browse the repository at this point in the history
### Description of the change

When packages don't have any readme file, currently the spinner is never
hidden. It gives the impression of something still unloaded, so the user
should remain in that page because sth else will be displayed soon.
However, this is not true: once `! isFetching`, if there is no readme...
it will never be available.

This PR passes the `isFetching` prop downstream so that the readme
renderer can decide what to display.

### Benefits

No more endless spinner when no readme is provided.

### Possible drawbacks

N/A


### Applicable issues

N/A

### Additional information


![image](https://github.com/vmware-tanzu/kubeapps/assets/11535726/37eb207f-84aa-49c4-97e9-4605d5f278d6)

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
  • Loading branch information
antgamdia committed Aug 9, 2023
1 parent 15173e4 commit 1473e94
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 35 deletions.
89 changes: 62 additions & 27 deletions dashboard/src/components/PackageHeader/PackageReadme.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import LoadingWrapper from "components/LoadingWrapper/LoadingWrapper";
import ReactMarkdown from "react-markdown";
import { HashLink as Link } from "react-router-hash-link";
import { defaultStore, mountWrapper } from "shared/specs/mountWrapper";
import PackageReadme from "./PackageReadme";
import PackageReadme, { IPackageReadmeProps } from "./PackageReadme";

const defaultProps = {
const defaultProps: IPackageReadmeProps = {
error: undefined,
readme: "",
isFetching: false,
};

const kubeActions = { ...actions.kube };
Expand All @@ -24,27 +25,46 @@ afterEach(() => {
actions.kube = { ...kubeActions };
});

it("behaves as a loading component", () => {
const wrapper = mountWrapper(defaultStore, <PackageReadme {...defaultProps} />);
it("behaves as a loading component if it's fetching with readme", () => {
const props: IPackageReadmeProps = {
...defaultProps,
isFetching: true,
readme: "foo",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

expect(wrapper.find(LoadingWrapper)).toExist();
});

it("behaves as a loading component if it's fetching without readme", () => {
const props: IPackageReadmeProps = {
...defaultProps,
isFetching: true,
readme: "",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

expect(wrapper.find(LoadingWrapper)).toExist();
});

it("renders the ReactMarkdown content is readme is present", () => {
const props = {
const props: IPackageReadmeProps = {
...defaultProps,
readme: "# Markdown Readme",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

const component = wrapper.find(ReactMarkdown);
expect(component.html()).toEqual('<h1 id="markdown-readme">Markdown Readme</h1>');
});

it("renders the ReactMarkdown content with github flavored markdown (table)", () => {
const props = {
const props: IPackageReadmeProps = {
...defaultProps,
readme: "|h1|h2|\n|-|-|\n|foo|bar|",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

const component = wrapper.find(ReactMarkdown);
expect(component.props()).toMatchObject({ children: props.readme });
expect(component.find("table th").first().text()).toBe("h1");
Expand All @@ -61,42 +81,57 @@ it("renders a not found error when error is set", () => {
expect(wrapper.text()).toContain("No README found");
});

it("renders a message if no readme is fetched", () => {
const props: IPackageReadmeProps = {
...defaultProps,
readme: "",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

expect(wrapper.text()).toContain("This package does not contain a README file.");
});

it("renders an alert when error is set", () => {
const wrapper = mountWrapper(defaultStore, <PackageReadme {...defaultProps} error={"Boom!"} />);
const props: IPackageReadmeProps = {
...defaultProps,
error: "Boom!",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

expect(wrapper.text()).toContain("Unable to fetch the package's README: Boom!");
});

it("renders the ReactMarkdown content adding IDs for the titles", () => {
const wrapper = mountWrapper(
defaultStore,
<PackageReadme {...defaultProps} readme="# _Markdown_ 'Readme_or_not'!" />,
);
const props: IPackageReadmeProps = {
...defaultProps,
readme: "# _Markdown_ 'Readme_or_not'!",
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

const component = wrapper.find("#markdown-readme_or_not");
expect(component).toExist();
});

it("renders the ReactMarkdown ignoring comments", () => {
const wrapper = mountWrapper(
defaultStore,
<PackageReadme
{...defaultProps}
readme={`<!-- This is a comment -->
This is text`}
/>,
);
const props: IPackageReadmeProps = {
...defaultProps,
readme: `<!-- This is a comment -->
This is text`,
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

const html = wrapper.html();
expect(html).toContain("This is text");
expect(html).not.toContain("This is a comment");
});

it("renders the ReactMarkdown content with hash links", () => {
const wrapper = mountWrapper(
defaultStore,
<PackageReadme
{...defaultProps}
readme={`[section 1](#section-1)
# Section 1`}
/>,
);
const props: IPackageReadmeProps = {
...defaultProps,
readme: `[section 1](#section-1)
# Section 1`,
};
const wrapper = mountWrapper(defaultStore, <PackageReadme {...props} />);

expect(wrapper.find(Link)).toExist();
});
17 changes: 10 additions & 7 deletions dashboard/src/components/PackageHeader/PackageReadme.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import HeadingRenderer from "../MarkdownRenderer/HeadingRenderer";
import LinkRenderer from "../MarkdownRenderer/LinkRenderer";
import TableRenderer from "../MarkdownRenderer/TableRenderer";

interface IPackageReadmeProps {
export interface IPackageReadmeProps {
error?: string;
readme?: string;
isFetching?: boolean;
}

function PackageReadme({ error, readme }: IPackageReadmeProps) {
function PackageReadme({ error, readme, isFetching }: IPackageReadmeProps) {
if (error) {
if (error.toLocaleLowerCase().includes("not found")) {
return (
Expand All @@ -33,10 +34,10 @@ function PackageReadme({ error, readme }: IPackageReadmeProps) {
<LoadingWrapper
className="margin-t-xxl"
loadingText="Fetching application README..."
loaded={!!readme}
loaded={!isFetching}
>
{readme && (
<div className="application-readme">
<div className="application-readme">
{readme ? (
<ReactMarkdown
remarkPlugins={[remarkGfm]}
components={{
Expand All @@ -53,8 +54,10 @@ function PackageReadme({ error, readme }: IPackageReadmeProps) {
>
{readme}
</ReactMarkdown>
</div>
)}
) : (
<p> This package does not contain a README file.</p>
)}
</div>
</LoadingWrapper>
);
}
Expand Down
6 changes: 5 additions & 1 deletion dashboard/src/components/PackageHeader/PackageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,11 @@ export default function PackageView() {
<AvailablePackageDetailExcerpt pkg={selectedPackage.availablePackageDetail} />
</Column>
<Column span={9}>
<PackageReadme readme={selectedPackage.readme} error={selectedPackage.readmeError} />
<PackageReadme
readme={selectedPackage.readme}
error={selectedPackage.readmeError}
isFetching={isFetching}
/>
<div className="after-readme-button">
<Link
to={app.apps.new(
Expand Down

0 comments on commit 1473e94

Please sign in to comment.