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.

Sign up for GitHub

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

Jump to bottom

DCOS-51377: Services Table: avoid rerender (except for status icons) #3796

Merged
merged 11 commits into from
Apr 10, 2019
Merged

DCOS-51377: Services Table: avoid rerender (except for status icons) #3796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e8ea54a
chore(services): remove tests for columns
Apr 10, 2019
2746426
perf(services): avoid rerenders for progress bars
Apr 9, 2019
19fd32e
perf(services): avoid rerenders for cpu column
Apr 10, 2019
75b42df
perf(services): avoid rerenders for disk column
Apr 10, 2019
708fdea
perf(services): avoid rerenders for gpu column
Apr 10, 2019
f1dd27c
perf(services): avoid rerenders for instances column
Apr 10, 2019
47c0ad3
perf(services): avoid rerenders for mem column
Apr 10, 2019
24e7078
perf(services): avoid rerenders for name column
Apr 10, 2019
60524c4
perf(services): avoid rerenders for version column
Apr 10, 2019
702400e
perf(services): avoid rerenders for region column
Apr 10, 2019
83efb61
Merge branch 'master' into danielmschmidt/DCOS-51377-second-try
juliangieseke Apr 10, 2019
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
14 changes: 7 additions & 7 deletions plugins/services/src/js/columns/ServicesTableCPUColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import Units from "#SRC/js/utils/Units";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

export const ServiceCPU = React.memo(({ resource }: { resource: string }) => (
<NumberCell>
<span>{Units.formatResource("cpus", resource)}</span>
</NumberCell>
));

export function cpuRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
const resource = service.getResources()[`cpus`];

return (
<NumberCell>
<span>{Units.formatResource("cpus", resource)}</span>
</NumberCell>
);
return <ServiceCPU resource={service.getResources()[`cpus`]} />;
}

export function cpuSorter(
Expand Down
14 changes: 7 additions & 7 deletions plugins/services/src/js/columns/ServicesTableDiskColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import Units from "#SRC/js/utils/Units";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

export const ServiceDisk = React.memo(({ resource }: { resource: string }) => (
<NumberCell>
<span>{Units.formatResource("disk", resource)}</span>
</NumberCell>
));

export function diskRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
const resource = service.getResources()[`disk`];

return (
<NumberCell>
<span>{Units.formatResource("disk", resource)}</span>
</NumberCell>
);
return <ServiceDisk resource={service.getResources()[`disk`]} />;
}

export function diskSorter(
Expand Down
14 changes: 7 additions & 7 deletions plugins/services/src/js/columns/ServicesTableGPUColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import Units from "#SRC/js/utils/Units";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

export const ServiceGPU = React.memo(({ resource }: { resource: string }) => (
<NumberCell>
<span>{Units.formatResource("gpus", resource)}</span>
</NumberCell>
));

export function gpuRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
const resource = service.getResources()[`gpus`];

return (
<NumberCell>
<span>{Units.formatResource("gpus", resource)}</span>
</NumberCell>
);
return <ServiceGPU resource={service.getResources()[`gpus`]} />;
}

export function gpuSorter(
Expand Down
63 changes: 39 additions & 24 deletions plugins/services/src/js/columns/ServicesTableInstancesColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,49 @@ import StringUtil from "#SRC/js/utils/StringUtil";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

export function instancesRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
const instancesCount = service.getInstancesCount();
const runningInstances = service.getRunningInstancesCount();
const overview =
runningInstances === instancesCount
? ` ${runningInstances}`
: ` ${runningInstances}/${instancesCount}`;
const ServiceInstances = React.memo(
({
instancesCount,
runningInstances
}: {
instancesCount: number;
runningInstances: number;
}) => {
const overview =
runningInstances === instancesCount
? ` ${runningInstances}`
: ` ${runningInstances}/${instancesCount}`;

const content = !Number.isInteger(instancesCount)
? EmptyStates.CONFIG_VALUE
: overview;
const content = !Number.isInteger(instancesCount)
? EmptyStates.CONFIG_VALUE
: overview;

// L10NTODO: Pluralize
const tooltipContent = (
<Trans render="span">
{runningInstances} {StringUtil.pluralize("instance", runningInstances)}{" "}
running out of {instancesCount}
</Trans>
);
// L10NTODO: Pluralize
const tooltipContent = (
<Trans render="span">
{runningInstances} {StringUtil.pluralize("instance", runningInstances)}{" "}
running out of {instancesCount}
</Trans>
);

return (
<NumberCell>
<Tooltip content={tooltipContent}>
<span>{content}</span>
</Tooltip>
</NumberCell>
);
}
);

export function instancesRenderer(
pierrebeitz marked this conversation as resolved.
Show resolved Hide resolved
service: Service | Pod | ServiceTree
): React.ReactNode {
return (
<NumberCell>
<Tooltip content={tooltipContent}>
<span>{content}</span>
</Tooltip>
</NumberCell>
<ServiceInstances
instancesCount={service.getInstancesCount()}
runningInstances={service.getRunningInstancesCount()}
/>
);
}

Expand Down
13 changes: 7 additions & 6 deletions plugins/services/src/js/columns/ServicesTableMemColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@ import Units from "#SRC/js/utils/Units";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

export const ServiceMem = React.memo(({ resource }: { resource: string }) => (
<NumberCell>
<span>{Units.formatResource("mem", resource)}</span>
</NumberCell>
));

export function memRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
const resource = service.getResources()[`mem`];
return (
<NumberCell>
<span>{Units.formatResource("mem", resource)}</span>
</NumberCell>
);
return <ServiceMem resource={service.getResources()[`mem`]} />;
}

export function memSorter(
Expand Down
92 changes: 66 additions & 26 deletions plugins/services/src/js/columns/ServicesTableNameColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,70 @@ import Pod from "../structs/Pod";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

const ServiceName = React.memo(
({
isFiltered,
id,
isGroup,
name,
image,
webUrl
}: {
isFiltered: boolean;
id: string;
isGroup: boolean;
name: string;
image: string | null;
webUrl: string | null;
}) => {
const serviceLink = isGroup
? `/services/overview/${id}`
: `/services/detail/${id}`;

return (
<TextCell>
<div className="service-table-heading flex-box flex-box-align-vertical-center table-cell-flex-box text-overflow">
<Link className="table-cell-icon" to={serviceLink}>
{getImage(image, isGroup)}
</Link>
<span className="table-cell-value table-cell-flex-box">
{getServiceLink(id, name, isGroup, isFiltered)}
{getOpenInNewWindowLink(webUrl)}
</span>
</div>
</TextCell>
);
}
);

export function nameRenderer(
isFiltered: boolean,
service: Service | Pod | ServiceTree
): React.ReactNode {
const id: string = encodeURIComponent(service.getId().toString());
const isGroup = service instanceof ServiceTree;
const serviceLink = isGroup
? `/services/overview/${id}`
: `/services/detail/${id}`;
// These do not work with instanceof ServiceTree due to TS
const image =
service instanceof Pod || service instanceof Service
? service.getImages()["icon-small"]
: null;
const webUrl =
service instanceof Pod || (service instanceof Service && hasWebUI(service))
? service.getWebURL()
: null;

return (
<TextCell>
<div className="service-table-heading flex-box flex-box-align-vertical-center table-cell-flex-box text-overflow">
<Link className="table-cell-icon" to={serviceLink}>
{getImage(service)}
</Link>
<span className="table-cell-value table-cell-flex-box">
{getServiceLink(service, isFiltered)}
{getOpenInNewWindowLink(service)}
</span>
</div>
</TextCell>
<ServiceName
Copy link
Contributor

Choose a reason for hiding this comment

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

'nit: I think it would have been possible to split this into ServiceName and GroupName component 🤔 not sure if I'm missing something and I think its not that important tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as before, I did not touch anything in this direction. I don't see that this is part of the scope of this ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least if our goal is still to merge this functionality today. If this goal changed we can also close this PR and use the #3792 one

id={encodeURIComponent(service.getId().toString())}
isGroup={service instanceof ServiceTree}
name={service.getName()}
image={image}
webUrl={webUrl}
isFiltered={isFiltered}
/>
);
}

function getImage(service: any): any {
if (service instanceof ServiceTree) {
function getImage(image: string | null, isGroup: boolean): React.ReactNode {
if (isGroup) {
// Get serviceTree image/icon
return (
<span className="icon-margin-right">
Expand All @@ -52,14 +89,17 @@ function getImage(service: any): any {
// Get service image/icon
return (
<span className="icon icon-mini icon-image-container icon-app-container icon-margin-right">
<img src={service.getImages()["icon-small"]} />
<img src={image as string} />
</span>
);
}

function getServiceLink(service: any, isFiltered: boolean): any {
const id = encodeURIComponent(service.getId());
const isGroup = service instanceof ServiceTree;
function getServiceLink(
id: string,
name: string,
isGroup: boolean,
isFiltered: boolean
): React.ReactNode {
const serviceLink = isGroup
? `/services/overview/${id}`
: `/services/detail/${id}`;
Expand All @@ -77,22 +117,22 @@ function getServiceLink(service: any, isFiltered: boolean): any {

return (
<Link className="table-cell-link-primary" to={serviceLink}>
{service.getName()}
{name}
</Link>
);
}

function getOpenInNewWindowLink(service: any): any {
function getOpenInNewWindowLink(webUrl: string | null): React.ReactNode {
// This might be a serviceTree and therefore we need this check
// And getWebURL might therefore not be available
if (!hasWebUI(service)) {
if (!webUrl) {
return null;
}

return (
<a
className="table-cell-icon table-display-on-row-hover"
href={service.getWebURL()}
href={webUrl}
target="_blank"
title="Open in a new window"
>
Expand Down
16 changes: 9 additions & 7 deletions plugins/services/src/js/columns/ServicesTableRegionColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ import ServiceTree from "../structs/ServiceTree";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";
import ServiceTableUtil from "../utils/ServiceTableUtil";

const ServiceRegion = React.memo(({ regions }: { regions: string }) => (
<TextCell>
<Tooltip elementTag="span" wrapText={true} content={regions}>
{regions}
</Tooltip>
</TextCell>
));

export function regionRendererFactory(localRegion: string | undefined) {
return (service: Service | Pod | ServiceTree) => {
let regions: string[] = service.getRegions();
Expand All @@ -20,13 +28,7 @@ export function regionRendererFactory(localRegion: string | undefined) {
regions.push("N/A");
}

return (
<TextCell>
<Tooltip elementTag="span" wrapText={true} content={regions.join(", ")}>
{regions.join(", ")}
</Tooltip>
</TextCell>
);
return <ServiceRegion regions={regions.join(", ")} />;
};
}
export function regionSorter(
Expand Down
25 changes: 20 additions & 5 deletions plugins/services/src/js/columns/ServicesTableVersionColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ import Service from "../structs/Service";
import ServiceTree from "../structs/ServiceTree";
import { SortDirection } from "plugins/services/src/js/types/SortDirection";

const ServiceVersion = React.memo(
({
rawVersion,
displayVersion
}: {
rawVersion: string;
displayVersion: string;
}) => (
<TextCell>
<Tooltip content={rawVersion} wrapText={true}>
{displayVersion}
</Tooltip>
</TextCell>
)
);

export function versionRenderer(
service: Service | Pod | ServiceTree
): React.ReactNode {
Expand All @@ -17,11 +33,10 @@ export function versionRenderer(
}

return (
<TextCell>
<Tooltip content={version.rawVersion} wrapText={true}>
{version.displayVersion}
</Tooltip>
</TextCell>
<ServiceVersion
rawVersion={version.rawVersion}
displayVersion={version.displayVersion}
/>
);
}

Expand Down
Loading