Skip to content
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
2 changes: 1 addition & 1 deletion frontend/__tests__/components/utils/firehose.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { Map as ImmutableMap } from 'immutable';
import Spy = jasmine.Spy;

import { Firehose } from '../../../public/components/utils/firehose';
import { FirehoseResource } from '../../../public/components/factory';
import { K8sKind, K8sResourceKindReference } from '../../../public/module/k8s';
import { PodModel, ServiceModel } from '../../../public/models';
import { FirehoseResource } from '../../../public/components/utils';

// TODO(alecmerdler): Use these once `Firehose` is converted to TypeScript
type FirehoseProps = {
Expand Down
12 changes: 6 additions & 6 deletions frontend/public/components/about-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,29 @@ import { connectToFlags } from '../reducers/features';
import { getBrandingDetails } from './masthead';
import { Firehose } from './utils';
import { ClusterVersionModel } from '../models';
import { ClusterVersionKind, referenceForModel, UpdateHistory } from '../module/k8s';
import { ClusterVersionKind, referenceForModel } from '../module/k8s';
import { k8sVersion } from '../module/status';
import { hasAvailableUpdates } from '../module/k8s/cluster-settings';
import { hasAvailableUpdates, getK8sGitVersion, getOpenShiftVersion } from '../module/k8s/cluster-settings';

const AboutModalItems: React.FC<AboutModalItemsProps> = ({closeAboutModal, cv}) => {
const [kubernetesVersion, setKubernetesVersion] = React.useState('');
React.useEffect(() => {
k8sVersion()
.then(({gitVersion}) => setKubernetesVersion(gitVersion))
.then(response => setKubernetesVersion(getK8sGitVersion(response) || '-'))
.catch(() => setKubernetesVersion('unknown'));
}, []);
const clusterID: string = _.get(cv, 'data.spec.clusterID');
const channel: string = _.get(cv, 'data.spec.channel');
const lastUpdate: UpdateHistory = _.get(cv, 'data.status.history[0]');
const openshiftVersion = getOpenShiftVersion(_.get(cv, 'data'));
return (
<React.Fragment>
{hasAvailableUpdates(cv.data) && <Alert className="co-about-modal__alert" variant="info" title={<React.Fragment>Update available. <Link to="/settings/cluster" onClick={closeAboutModal}>View Cluster Settings</Link></React.Fragment>} />}
<TextContent>
<TextList component="dl">
{lastUpdate && (
{openshiftVersion && (
<React.Fragment>
<TextListItem component="dt">OpenShift Version</TextListItem>
<TextListItem component="dd">{lastUpdate.state === 'Partial' ? `Updating to ${lastUpdate.version}` : lastUpdate.version}</TextListItem>
<TextListItem component="dd">{openshiftVersion}</TextListItem>
</React.Fragment>
)}
<TextListItem component="dt">Kubernetes Version</TextListItem>
Expand Down
32 changes: 32 additions & 0 deletions frontend/public/components/dashboard/details-card/detail-item.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import * as React from 'react';
import * as _ from 'lodash-es';
import { OverlayTrigger, Tooltip } from 'patternfly-react';

import { LoadingInline } from '../../utils';

export const DetailItem: React.FC<DetailItemProps> = React.memo(({ title, value, isLoading }) => {
const description = value ? (
<OverlayTrigger
overlay={<Tooltip id={_.uniqueId('tooltip-for-')}>{value}</Tooltip>}
Copy link
Member

Choose a reason for hiding this comment

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

How is id used for Tooltip? I don't see where it's referenced. If it's not required, we should leave it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

placement="top"
trigger={['hover', 'focus']}
rootClose={false}
>
<span>{value}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Can this be {value} or if necessary <React.Fragment>{value}</React.Fragment> to avoid the unnecessary span?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no :/ using {value} results in error in runtime and <React.Fragment>{value}</React.Fragment> does not work either - no error, but tooltip does not appear when hovering over

Copy link
Member

Choose a reason for hiding this comment

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

That seems like a Patternfly bug :/

</OverlayTrigger>
) : (
'-'
);
return (
<React.Fragment>
<dt className="co-details-card__item-title">{title}</dt>
<dd className="co-details-card__item-value">{isLoading ? <LoadingInline /> : description}</dd>
Copy link
Member

Choose a reason for hiding this comment

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

I haven't seen how this looks, but I suspect having a loading icon for every detail item is too much animation.

Again we should consider skeleton while the widget is loading.

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, there is going to be quite a few loadings through the Overview Tab. We will be adding skeletons as a follow up.

</React.Fragment>
);
});

type DetailItemProps = {
title: string;
value?: string;
isLoading: boolean;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as React from 'react';

export const DetailsBody: React.FC<DetailsBodyProps> = ({ children }) => (
<dl className="co-details-card__body">{children}</dl>
);

type DetailsBodyProps = {
children: React.ReactNode;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
.co-details-card {
margin-bottom: 1rem;
}

.co-details-card__body {
display: flex;
flex-wrap: wrap;
}

.co-details-card__item-title {
flex-basis: 100%;
font-size: 0.875rem;
text-transform: capitalize;
}

.co-details-card__item-value {
flex-basis: 100%;
font-size: 0.875rem;
margin-bottom: 8px;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
2 changes: 2 additions & 0 deletions frontend/public/components/dashboard/details-card/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './detail-item';
export * from './details-body';
29 changes: 22 additions & 7 deletions frontend/public/components/dashboards-page/dashboards.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import * as React from 'react';
import { RouteComponentProps } from 'react-router-dom';
import { connect } from 'react-redux';

import { OverviewDashboard } from './overview-dashboard/overview-dashboard';
import { HorizontalNav, PageHeading } from '../utils';
import { HorizontalNav, PageHeading, LoadingBox } from '../utils';

const tabs = [
{
Expand All @@ -12,9 +13,23 @@ const tabs = [
},
];

export const DashboardsPage: React.FC<RouteComponentProps> = ({ match }) => (
<React.Fragment>
<PageHeading title="Dashboards" detail={true} />
<HorizontalNav match={match} pages={tabs} noStatusBox />
</React.Fragment>
);
const _DashboardsPage: React.FC<DashboardsPageProps> = ({ match, kindsInFlight }) => {
return kindsInFlight
? <LoadingBox />
: (
<>
<PageHeading title="Dashboards" detail={true} />
<HorizontalNav match={match} pages={tabs} noStatusBox />
</>
);
};

const mapStateToProps = ({k8s}) => ({
kindsInFlight: k8s.getIn(['RESOURCES', 'inFlight']),
});

export const DashboardsPage = connect(mapStateToProps)(_DashboardsPage);

type DashboardsPageProps = RouteComponentProps & {
kindsInFlight: boolean;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import * as React from 'react';
import * as _ from 'lodash-es';

import {
DashboardCard,
DashboardCardBody,
DashboardCardHeader,
DashboardCardTitle,
} from '../../dashboard/dashboard-card';
import { DetailsBody, DetailItem } from '../../dashboard/details-card';
import { withDashboardResources, DashboardItemProps } from '../with-dashboard-resources';
import { InfrastructureModel, ClusterVersionModel } from '../../../models';
import { referenceForModel, K8sResourceKind, getOpenShiftVersion, getK8sGitVersion, getClusterName, ClusterVersionKind } from '../../../module/k8s';
import { FLAGS } from '../../../const';
import { flagPending, connectToFlags } from '../../../reducers/features';
import { FirehoseResource } from '../../utils';

const getInfrastructurePlatform = (infrastructure: K8sResourceKind): string => _.get(infrastructure, 'status.platform');

const clusterVersionResource: FirehoseResource = {
kind: referenceForModel(ClusterVersionModel),
namespaced: false,
name: 'version',
isList: false,
prop: 'cv',
};

const infrastructureResource: FirehoseResource = {
kind: referenceForModel(InfrastructureModel),
namespaced: false,
name: 'cluster',
isList: false,
prop: 'infrastructure',
};

export const DetailsCard_: React.FC<DetailsCardProps> = ({
watchURL,
stopWatchURL,
watchK8sResource,
stopWatchK8sResource,
resources,
urlResults,
flags,
}) => {
const openshiftFlag = flags[FLAGS.OPENSHIFT];
React.useEffect(() => {
if (flagPending(openshiftFlag)) {
return;
}
if (openshiftFlag) {
watchK8sResource(clusterVersionResource);
watchK8sResource(infrastructureResource);
return () => {
stopWatchK8sResource(clusterVersionResource);
stopWatchK8sResource(infrastructureResource);
};
}
watchURL('version');
return () => {
stopWatchURL('version');
};
}, [openshiftFlag, watchK8sResource, stopWatchK8sResource, watchURL, stopWatchURL]);

const clusterVersion = _.get(resources, 'cv');
Copy link
Member

Choose a reason for hiding this comment

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

This seems cleaner to me:

Suggested change
const clusterVersion = _.get(resources, 'cv');
const clusterVersion = _.get(resources, 'cv.data') as ClusterVersionKind;
// ...
const openshiftVersion = getOpenShiftVersion(clusterVersion);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript complains Conversion of type 'FirehoseResult' to type 'ClusterVersionKind' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. . It thinks that cv.data is FirehoseResult but in reality it is K8sResourceKind | K8sResourceKind[] :/ Splitting the expression in a way I do does not result in error. Seems like a bug in TS.

Copy link
Member

Choose a reason for hiding this comment

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

That seems weird. Is this a bug in the lodash typings? How does TypeScript know the type returned by _.get?

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'd say that is an issue of @types/lodash-es but Im not very familiar with it. I tried updating to latest version (4.17.3, console is using 4.17.0) but it didnt help :/

const clusterVersionLoaded = _.get(clusterVersion, 'loaded', false);
const openshiftVersion = getOpenShiftVersion(_.get(clusterVersion, 'data') as ClusterVersionKind);

const infrastructure = _.get(resources, 'infrastructure');
const infrastructureLoaded = _.get(infrastructure, 'loaded', false);
const infrastructureData = _.get(infrastructure, 'data') as K8sResourceKind;


const kubernetesVersionResponse = urlResults.getIn(['version', 'result']);

return (
<DashboardCard className="co-details-card">
<DashboardCardHeader>
<DashboardCardTitle>Details</DashboardCardTitle>
</DashboardCardHeader>
<DashboardCardBody isLoading={flagPending(openshiftFlag)}>
<DetailsBody>
{openshiftFlag ? (
<>
<DetailItem
key="name"
title="Name"
value={getClusterName()}
isLoading={false}
/>
<DetailItem
key="provider"
title="Provider"
value={getInfrastructurePlatform(infrastructureData)}
isLoading={!infrastructureLoaded}
/>
<DetailItem
key="openshift"
title="OpenShift version"
value={openshiftVersion}
isLoading={!clusterVersionLoaded}
/>
</>
) : (
<DetailItem
key="kubernetes"
title="Kubernetes version"
value={getK8sGitVersion(kubernetesVersionResponse)}
isLoading={!kubernetesVersionResponse}
/>
)}
</DetailsBody>
</DashboardCardBody>
</DashboardCard>
);
};

type DetailsCardProps = DashboardItemProps & {
flags: {[FLAGS.OPENSHIFT]: boolean};
}

export const DetailsCard = withDashboardResources(connectToFlags(FLAGS.OPENSHIFT)(DetailsCard_));
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const fetchK8sHealth = async(url) => {
return response.text();
};

const _HealthCard: React.FC<HealthProps> = ({
const HealthCard_: React.FC<HealthProps> = ({
watchURL,
stopWatchURL,
watchPrometheus,
Expand Down Expand Up @@ -153,7 +153,7 @@ const _HealthCard: React.FC<HealthProps> = ({
);
};

export const HealthCard = withDashboardResources(connect(mapStateToProps)(_HealthCard));
export const HealthCard = withDashboardResources(connect(mapStateToProps)(HealthCard_));

type ClusterHealth = {
state: HealthState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@ import * as React from 'react';

import { Dashboard, DashboardGrid } from '../../dashboard';
import { HealthCard } from './health-card';
import { DetailsCard } from './details-card';

export const OverviewDashboard: React.FC<{}> = () => {
const mainCards = [
<HealthCard key="health" />,
];

const leftCards = [
<DetailsCard key="details" />,
];

return (
<Dashboard>
<DashboardGrid mainCards={mainCards} />
<DashboardGrid mainCards={mainCards} leftCards={leftCards} />
</Dashboard>
);
};
Loading