Improve ui performance

Move repository overview extension to avoid waiting for repositories. Also improve the extension points api to allow predicates without props.

Co-authored-by: Konstantin Schaper <konstantin.schaper@cloudogu.com>
This commit is contained in:
Eduard Heimbuch
2023-06-22 13:41:17 +02:00
parent 1c687e7279
commit 6bb9024504
6 changed files with 124 additions and 63 deletions

View File

@@ -0,0 +1,4 @@
- type: changed
description: Optimize ui performance for repository overview
- type: changed
description: Enhance extensions name logic by allow bind options

View File

@@ -106,7 +106,7 @@ describe("binder tests", () => {
type TestExtensionPointA = ExtensionPointDefinition<"test.extension.a", number, undefined>;
type TestExtensionPointB = ExtensionPointDefinition<"test.extension.b", number, { testProp: boolean[] }>;
binder.bind<TestExtensionPointA>("test.extension.a", 2, () => false);
binder.bind<TestExtensionPointA>("test.extension.a", 2, () => true);
const binderExtensionA = binder.getExtension<TestExtensionPointA>("test.extension.a");
expect(binderExtensionA).not.toBeNull();
binder.bind<TestExtensionPointB>("test.extension.b", 2);
@@ -114,7 +114,7 @@ describe("binder tests", () => {
testProp: [true, false],
});
expect(binderExtensionsB).toHaveLength(1);
binder.bind("test.extension.c", 2, () => false);
binder.bind("test.extension.c", 2, () => true);
const binderExtensionC = binder.getExtension("test.extension.c");
expect(binderExtensionC).not.toBeNull();
});
@@ -126,7 +126,7 @@ describe("binder tests", () => {
binder.bind<TestExtensionPointA>(
"test.extension.a",
() => <h1>Hello world</h1>,
() => false
() => true
);
const binderExtensionA = binder.getExtension<TestExtensionPointA>("test.extension.a");
expect(binderExtensionA).not.toBeNull();

View File

@@ -53,8 +53,8 @@ export type BindOptions<Props> = {
priority?: number;
};
function isBindOptions<Props>(input?: Predicate<Props> | BindOptions<Props>): input is BindOptions<Props> {
return typeof input !== "function" && typeof input === "object";
function isBindOptions<Props>(input?: string | Predicate<Props> | BindOptions<Props>): input is BindOptions<Props> {
return typeof input !== "string" && typeof input !== "function" && typeof input === "object";
}
/**
@@ -105,11 +105,25 @@ export class Binder {
extension: E["type"],
options?: BindOptions<E["props"]>
): void;
/**
* Binds an extension to the extension point.
*
* @param extensionPoint name of extension point
* @param extension provided extension
* @param predicate to decide if the extension gets rendered for the given props
* @param options object with additional settings
*/
bind<E extends ExtensionPointDefinition<string, unknown, any>>(
extensionPoint: E["name"],
extension: E["type"],
predicate?: Predicate<E["props"]>,
options?: BindOptions<E["props"]>
): void;
bind<E extends ExtensionPointDefinition<string, unknown, any>>(
extensionPoint: E["name"],
extension: E["type"],
predicateOrOptions?: Predicate<E["props"]> | BindOptions<E["props"]>,
extensionName?: string
extensionNameOrOptions?: string | BindOptions<E["props"]>
) {
let predicate: Predicate<E["props"]> = () => true;
let priority = 0;
@@ -118,13 +132,23 @@ export class Binder {
predicate = predicateOrOptions.predicate;
}
if (predicateOrOptions.extensionName) {
extensionName = predicateOrOptions.extensionName;
extensionNameOrOptions = predicateOrOptions.extensionName;
}
if (typeof predicateOrOptions.priority === "number") {
priority = predicateOrOptions.priority;
}
} else if (predicateOrOptions) {
predicate = predicateOrOptions;
if (isBindOptions(extensionNameOrOptions)) {
if (typeof extensionNameOrOptions.priority === "number") {
priority = extensionNameOrOptions.priority;
}
if (extensionNameOrOptions?.extensionName) {
extensionNameOrOptions = extensionNameOrOptions.extensionName;
} else {
extensionNameOrOptions = undefined;
}
}
}
if (!this.extensionPoints[extensionPoint]) {
this.extensionPoints[extensionPoint] = [];
@@ -132,7 +156,7 @@ export class Binder {
const registration = {
predicate,
extension,
extensionName: extensionName ? extensionName : "",
extensionName: extensionNameOrOptions ? extensionNameOrOptions : "",
priority,
} as ExtensionRegistration<E["props"], E["type"]>;
this.extensionPoints[extensionPoint].push(registration);
@@ -186,9 +210,7 @@ export class Binder {
props?: E["props"]
): Array<E["type"]> {
let registrations = this.extensionPoints[extensionPoint] || [];
if (props) {
registrations = registrations.filter((reg) => reg.predicate(props));
}
registrations.sort(this.sortExtensions);
return registrations.map((reg) => reg.extension);
}

View File

@@ -234,7 +234,7 @@ export type RepositoryOverviewTop = RenderableExtensionPointDefinition<
"repository.overview.top",
{
page: number;
search: string;
search?: string;
namespace?: string;
}
>;

View File

@@ -27,40 +27,22 @@ import { NamespaceCollection, Repository } from "@scm-manager/ui-types";
import groupByNamespace from "./groupByNamespace";
import RepositoryGroupEntry from "./RepositoryGroupEntry";
import { ExtensionPoint, extensionPoints } from "@scm-manager/ui-extensions";
import { KeyboardIterator, KeyboardSubIterator } from "@scm-manager/ui-shortcuts";
type Props = {
repositories: Repository[];
namespaces: NamespaceCollection;
page: number;
search: string;
namespace?: string;
};
class RepositoryList extends React.Component<Props> {
render() {
const { repositories, namespaces, namespace, page, search } = this.props;
const { repositories, namespaces } = this.props;
const groups = groupByNamespace(repositories, namespaces);
return (
<div className="content">
<KeyboardIterator>
<KeyboardSubIterator>
<ExtensionPoint<extensionPoints.RepositoryOverviewTop>
name="repository.overview.top"
renderAll={true}
props={{
page,
search,
namespace,
}}
/>
</KeyboardSubIterator>
{groups.map((group) => {
return <RepositoryGroupEntry group={group} key={group.name} />;
})}
</KeyboardIterator>
</div>
);
}

View File

@@ -27,7 +27,9 @@ import { useTranslation } from "react-i18next";
import {
CreateButton,
devices,
ErrorNotification,
LinkPaginator,
Loading,
Notification,
OverviewPageActions,
Page,
@@ -39,6 +41,8 @@ import { useNamespaceAndNameContext, useNamespaces, useRepositories } from "@scm
import { NamespaceCollection, RepositoryCollection } from "@scm-manager/ui-types";
import { binder, ExtensionPoint, extensionPoints, useBinder } from "@scm-manager/ui-extensions";
import styled from "styled-components";
import { KeyboardIterator, KeyboardSubIterator } from "@scm-manager/ui-shortcuts";
import classNames from "classnames";
const StickyColumn = styled.div`
align-self: flex-start;
@@ -100,32 +104,68 @@ const useOverviewData = () => {
type RepositoriesProps = {
namespaces?: NamespaceCollection;
repositories?: RepositoryCollection;
search: string;
search?: string;
page: number;
namespace?: string;
isLoading?: boolean;
error?: Error;
hasTopExtension?: boolean;
};
const Repositories: FC<RepositoriesProps> = ({ namespaces, namespace, repositories, search, page }) => {
const Repositories: FC<RepositoriesProps> = ({
namespaces,
repositories,
hasTopExtension,
search,
page,
error,
isLoading,
}) => {
const [t] = useTranslation("repos");
if (namespaces && repositories) {
let header;
if (hasTopExtension) {
header = (
<div className={classNames("is-flex", "is-align-items-center", "is-size-6", "has-text-weight-bold", "p-3")}>
{t("overview.title")}
</div>
);
}
if (error) {
return (
<>
{header}
<ErrorNotification error={error} />
</>
);
} else if (isLoading) {
return (
<>
{header}
<Loading />
</>
);
} else if (namespaces && repositories) {
if (repositories._embedded && repositories._embedded.repositories.length > 0) {
return (
<>
<RepositoryList
repositories={repositories._embedded.repositories}
namespaces={namespaces}
page={page}
search={search}
namespace={namespace}
/>
<RepositoryList repositories={repositories._embedded.repositories} namespaces={namespaces} />
<LinkPaginator collection={repositories} page={page} filter={search} />
</>
);
} else {
return <Notification type="info">{t("overview.noRepositories")}</Notification>;
return (
<>
{header}
<Notification type="info">{t("overview.noRepositories")}</Notification>
</>
);
}
} else {
return <Notification type="info">{t("overview.invalidNamespace")}</Notification>;
return (
<>
{header}
<Notification type="info">{t("overview.invalidNamespace")}</Notification>
</>
);
}
};
@@ -146,8 +186,6 @@ const Overview: FC = () => {
};
}, [namespace, context]);
const extensions = binder.getExtensions<extensionPoints.RepositoryOverviewLeft>("repository.overview.left");
// we keep the create permission in the state,
// because it does not change during searching or paging
// and we can avoid bouncing of search bar elements
@@ -180,9 +218,7 @@ const Overview: FC = () => {
}
};
const hasExtensions = extensions.length > 0;
const createLink = namespace ? `/repos/create/?namespace=${namespace}`: "/repos/create/";
const createLink = namespace ? `/repos/create/?namespace=${namespace}` : "/repos/create/";
return (
<Page
documentTitle={t("overview.title")}
@@ -196,23 +232,40 @@ const Overview: FC = () => {
{t("overview.subtitle")}
</ExtensionPoint>
}
loading={isLoading}
error={error}
>
<div className="columns">
{hasExtensions ? (
{binder.hasExtension<extensionPoints.RepositoryOverviewLeft>("repository.overview.left") ? (
<StickyColumn className="column is-one-third">
{extensions.map((extension) => React.createElement(extension))}
{<ExtensionPoint<extensionPoints.RepositoryOverviewLeft> name="repository.overview.left" renderAll />}
</StickyColumn>
) : null}
<div className="column is-clipped">
<KeyboardIterator>
<KeyboardSubIterator>
<ExtensionPoint<extensionPoints.RepositoryOverviewTop>
name="repository.overview.top"
renderAll={true}
props={{
page,
search,
namespace,
}}
/>
</KeyboardSubIterator>
<Repositories
namespaces={namespaces}
namespace={namespace}
repositories={repositories}
search={search}
page={page}
isLoading={isLoading}
error={error}
hasTopExtension={binder.hasExtension<extensionPoints.RepositoryOverviewTop>("repository.overview.top", {
page,
search,
namespace,
})}
/>
</KeyboardIterator>
{showCreateButton ? <CreateButton label={t("overview.createButton")} link={createLink} /> : null}
</div>
</div>