Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds placeholder-style loading to the ServiceItems #932

Merged
merged 4 commits into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ exports[`when apps available matches the snapshot 1`] = `
<main>
<LoadingWrapper
loaded={true}
type={0}
>
<div>
<CardGrid>
Expand Down Expand Up @@ -95,6 +96,7 @@ exports[`when error present matches the snapshot 1`] = `
<main>
<LoadingWrapper
loaded={true}
type={0}
>
<ErrorSelector
action="list"
Expand Down Expand Up @@ -130,6 +132,7 @@ exports[`when error present renders a generic error message 1`] = `
<main>
<LoadingWrapper
loaded={true}
type={0}
>
<ErrorSelector
action="list"
Expand Down Expand Up @@ -196,6 +199,7 @@ exports[`when fetched but not apps available matches the snapshot 1`] = `
<main>
<LoadingWrapper
loaded={true}
type={0}
>
<MessageAlertPage
header="Supercharge your Kubernetes cluster"
Expand Down Expand Up @@ -267,6 +271,7 @@ exports[`while fetching apps loading spinner matches the snapshot 1`] = `
<main>
<LoadingWrapper
loaded={false}
type={0}
>
<div />
</LoadingWrapper>
Expand Down Expand Up @@ -328,6 +333,7 @@ exports[`while fetching apps matches the snapshot 1`] = `
<main>
<LoadingWrapper
loaded={false}
type={0}
>
<div />
</LoadingWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports[`when fetching ingresses or services loading spinner matches the snapsho
<LoadingWrapper
loaded={false}
size="small"
type={0}
>
<table>
<thead>
Expand Down Expand Up @@ -37,6 +38,7 @@ exports[`when fetching ingresses or services loading spinner matches the snapsho
<LoadingWrapper
loaded={false}
size="small"
type={0}
>
<p>
The current application does not expose a public URL.
Expand All @@ -53,6 +55,7 @@ exports[`when the app contain ingresses should show the table with available ing
<LoadingWrapper
loaded={true}
size="small"
type={0}
>
<table>
<thead>
Expand Down Expand Up @@ -96,6 +99,7 @@ exports[`when the app contain services and ingresses should show the table with
<LoadingWrapper
loaded={true}
size="small"
type={0}
>
<table>
<thead>
Expand Down Expand Up @@ -152,6 +156,7 @@ exports[`when the app contain services should show the table if any service is a
<LoadingWrapper
loaded={true}
size="small"
type={0}
>
<table>
<thead>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports[`renders a deployment ready 1`] = `
<LoadingWrapper
loaded={true}
size="small"
type={0}
>
<table>
<thead>
Expand Down Expand Up @@ -53,6 +54,7 @@ exports[`when fetching deployments loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
size="small"
type={0}
>
<table>
<thead>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports[`renders a table with a secret 1`] = `
<LoadingWrapper
loaded={true}
size="small"
type={0}
>
<table>
<thead>
Expand Down Expand Up @@ -57,6 +58,7 @@ exports[`when fetching secrets loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
size="small"
type={0}
>
<table>
<thead>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from "react";
import { AlertTriangle } from "react-feather";

import LoadingWrapper from "../../../components/LoadingWrapper";
import LoadingWrapper, { LoaderType } from "../../../components/LoadingWrapper";
import { IKubeItem, IResource, IServiceSpec, IServiceStatus } from "../../../shared/types";

interface IServiceItemProps {
Expand Down Expand Up @@ -29,7 +29,7 @@ class ServiceItem extends React.Component<IServiceItemProps> {
if (service === undefined || service.isFetching) {
return (
<td colSpan={4}>
<LoadingWrapper size="inline-small" />
<LoadingWrapper type={LoaderType.Placeholder} />
</td>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ exports[`when fetching services loading spinner matches the snapshot 1`] = `
>
<LoadingWrapper
loaded={false}
size="inline-small"
type={1}
/>
</td>
</tr>
Expand All @@ -22,7 +22,7 @@ exports[`when fetching services loading spinner matches the snapshot 2`] = `
>
<LoadingWrapper
loaded={false}
size="inline-small"
type={1}
/>
</td>
</tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
exports[`when name or namespace do not exist loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;

exports[`when name or namespace do not exist loading spinner matches the snapshot 2`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
exports[`AppViewComponent when app info is null loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ exports[`renderization when charts available should render the list of charts 1`
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<CardGrid>
<CatalogItem
Expand Down Expand Up @@ -65,6 +66,7 @@ exports[`renderization when fetching apps loading spinner matches the snapshot 1
</PageHeader>
<LoadingWrapper
loaded={false}
type={0}
>
<CardGrid />
</LoadingWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
exports[`when readme is not present loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
exports[`when fetching is false but no chart is available loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;

exports[`when fetching is true and chart is available loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ exports[`if the service broker is not installed shows a warning message 1`] = `
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<ServiceCatalogNotInstalledAlert />
</LoadingWrapper>
Expand All @@ -28,6 +29,7 @@ exports[`when all the brokers are loaded shows a forbiden (fetch) error if it ex
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<main>
<ErrorSelector
Expand Down Expand Up @@ -125,6 +127,7 @@ exports[`when all the brokers are loaded shows a forbiden (resync) error if it e
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<main>
<ErrorSelector
Expand Down Expand Up @@ -350,6 +353,7 @@ exports[`when all the brokers are loaded shows a warning to install no service b
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<main>
<ServiceBrokersNotFoundAlert>
Expand Down Expand Up @@ -443,6 +447,7 @@ exports[`when all the brokers are loaded when there are no errors, renders the b
</PageHeader>
<LoadingWrapper
loaded={true}
type={0}
>
<main>
<CardGrid
Expand Down Expand Up @@ -483,6 +488,7 @@ exports[`while fetching brokers loading spinner matches the snapshot 1`] = `
</PageHeader>
<LoadingWrapper
loaded={false}
type={0}
>
<main>
<ServiceBrokersNotFoundAlert />
Expand All @@ -502,6 +508,7 @@ exports[`while fetching brokers matches the snapshot 1`] = `
</PageHeader>
<LoadingWrapper
loaded={false}
type={0}
>
<main>
<ServiceBrokersNotFoundAlert />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ it("renders the full DeploymentForm", () => {
expect(wrapper).toMatchSnapshot();
});

it("renders a release name by default, relying in Monickers output", () => {
it("renders a release name by default, relying in Monickers output", () => {
const versions = [{ id: "foo", attributes: { version: "1.2.3" } }] as IChartVersion[];
monikerChooseMock.mockImplementationOnce(() => "foo").mockImplementationOnce(() => "bar");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`loading spinner matches the snapshot 1`] = `
<LoadingWrapper
loaded={false}
type={0}
/>
`;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.LoadingPlaceholder {
height: 1em;
background-color: #f1f1f1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this is the minimum amount of CSS that we need? Do we need the background color set here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is the grey background colour that the lighter one animates over

position: relative;
&:after {
content: "";
position: absolute;
width: 100%;
height: 1em;
background: linear-gradient(
90deg,
rgba(0, 0, 0, 0),
rgba(255, 255, 255, 25%),
rgba(0, 0, 0, 0)
);
transform: translateX(-30%);
animation: loading 1.5s infinite;
}
}

@keyframes loading {
100% {
transform: translateX(100%);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { shallow } from "enzyme";
import * as React from "react";
import LoadingPlaceholder from "./LoadingPlaceholder";

it("matches the snapshot", () => {
const wrapper = shallow(<LoadingPlaceholder />);
expect(wrapper).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as React from "react";

import "./LoadingPlaceholder.css";

// Renders a placeholder loader style
// Based on https://kyusuf.com/post/fake-it-til-you-make-it-css/
const LoadingPlaceholder: React.SFC = props => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing tests, even if it is a simple snapshot one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that the LoadingSpinner didn't have tests, but I can add them for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a lot to test but I guess it's something nice to have

return <div className="LoadingPlaceholder" />;
};

export default LoadingPlaceholder;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`matches the snapshot 1`] = `
<div
className="LoadingPlaceholder"
/>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import LoadingPlaceholder from "./LoadingPlaceholder";

export default LoadingPlaceholder;
11 changes: 10 additions & 1 deletion dashboard/src/components/LoadingWrapper/LoadingWrapper.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { shallow } from "enzyme";
import * as React from "react";
import LoadingWrapper from ".";
import LoadingWrapper, { LoaderType } from ".";
import LoadingSpinner from "../LoadingSpinner";
import LoadingPlaceholder from "./LoadingPlaceholder";

let props = {} as any;

Expand Down Expand Up @@ -60,3 +61,11 @@ describe("when loaded is true", () => {
expect(wrapper.find(ChildrenComponent)).toExist();
});
});

describe("loader types", () => {
it("renders the LoadingPlaceholder type when specified", () => {
const wrapper = renderComponent({ type: LoaderType.Placeholder });
expect(wrapper).toMatchSnapshot();
expect(wrapper.find(LoadingPlaceholder)).toExist();
});
});
Loading