From 6bb90245049e2f471fe2a5d11bf42e4507845c12 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 22 Jun 2023 13:41:17 +0200 Subject: [PATCH] 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 --- gradle/changelog/ui_performance.yaml | 4 + scm-ui/ui-extensions/src/binder.test.tsx | 6 +- scm-ui/ui-extensions/src/binder.ts | 38 ++++-- scm-ui/ui-extensions/src/extensionPoints.ts | 2 +- .../repos/components/list/RepositoryList.tsx | 26 +--- .../src/repos/containers/Overview.tsx | 111 +++++++++++++----- 6 files changed, 124 insertions(+), 63 deletions(-) create mode 100644 gradle/changelog/ui_performance.yaml diff --git a/gradle/changelog/ui_performance.yaml b/gradle/changelog/ui_performance.yaml new file mode 100644 index 0000000000..370e61a2d8 --- /dev/null +++ b/gradle/changelog/ui_performance.yaml @@ -0,0 +1,4 @@ +- type: changed + description: Optimize ui performance for repository overview +- type: changed + description: Enhance extensions name logic by allow bind options diff --git a/scm-ui/ui-extensions/src/binder.test.tsx b/scm-ui/ui-extensions/src/binder.test.tsx index 86a38017e6..55d7b6fda2 100644 --- a/scm-ui/ui-extensions/src/binder.test.tsx +++ b/scm-ui/ui-extensions/src/binder.test.tsx @@ -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("test.extension.a", 2, () => false); + binder.bind("test.extension.a", 2, () => true); const binderExtensionA = binder.getExtension("test.extension.a"); expect(binderExtensionA).not.toBeNull(); binder.bind("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( "test.extension.a", () =>

Hello world

, - () => false + () => true ); const binderExtensionA = binder.getExtension("test.extension.a"); expect(binderExtensionA).not.toBeNull(); diff --git a/scm-ui/ui-extensions/src/binder.ts b/scm-ui/ui-extensions/src/binder.ts index a41de04a14..7bb79d4a59 100644 --- a/scm-ui/ui-extensions/src/binder.ts +++ b/scm-ui/ui-extensions/src/binder.ts @@ -53,8 +53,8 @@ export type BindOptions = { priority?: number; }; -function isBindOptions(input?: Predicate | BindOptions): input is BindOptions { - return typeof input !== "function" && typeof input === "object"; +function isBindOptions(input?: string | Predicate | BindOptions): input is BindOptions { + return typeof input !== "string" && typeof input !== "function" && typeof input === "object"; } /** @@ -105,11 +105,25 @@ export class Binder { extension: E["type"], options?: BindOptions ): 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>( + extensionPoint: E["name"], + extension: E["type"], + predicate?: Predicate, + options?: BindOptions + ): void; bind>( extensionPoint: E["name"], extension: E["type"], predicateOrOptions?: Predicate | BindOptions, - extensionName?: string + extensionNameOrOptions?: string | BindOptions ) { let predicate: Predicate = () => 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; this.extensionPoints[extensionPoint].push(registration); @@ -186,9 +210,7 @@ export class Binder { props?: E["props"] ): Array { let registrations = this.extensionPoints[extensionPoint] || []; - if (props) { - registrations = registrations.filter((reg) => reg.predicate(props)); - } + registrations = registrations.filter((reg) => reg.predicate(props)); registrations.sort(this.sortExtensions); return registrations.map((reg) => reg.extension); } diff --git a/scm-ui/ui-extensions/src/extensionPoints.ts b/scm-ui/ui-extensions/src/extensionPoints.ts index 79c97a0cca..d9ff84dd21 100644 --- a/scm-ui/ui-extensions/src/extensionPoints.ts +++ b/scm-ui/ui-extensions/src/extensionPoints.ts @@ -234,7 +234,7 @@ export type RepositoryOverviewTop = RenderableExtensionPointDefinition< "repository.overview.top", { page: number; - search: string; + search?: string; namespace?: string; } >; diff --git a/scm-ui/ui-webapp/src/repos/components/list/RepositoryList.tsx b/scm-ui/ui-webapp/src/repos/components/list/RepositoryList.tsx index f32398aee7..57d45cc411 100644 --- a/scm-ui/ui-webapp/src/repos/components/list/RepositoryList.tsx +++ b/scm-ui/ui-webapp/src/repos/components/list/RepositoryList.tsx @@ -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 { render() { - const { repositories, namespaces, namespace, page, search } = this.props; + const { repositories, namespaces } = this.props; const groups = groupByNamespace(repositories, namespaces); return (
- - - - name="repository.overview.top" - renderAll={true} - props={{ - page, - search, - namespace, - }} - /> - - {groups.map((group) => { - return ; - })} - + {groups.map((group) => { + return ; + })}
); } diff --git a/scm-ui/ui-webapp/src/repos/containers/Overview.tsx b/scm-ui/ui-webapp/src/repos/containers/Overview.tsx index 975f5c76bd..330c7bbe68 100644 --- a/scm-ui/ui-webapp/src/repos/containers/Overview.tsx +++ b/scm-ui/ui-webapp/src/repos/containers/Overview.tsx @@ -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 = ({ namespaces, namespace, repositories, search, page }) => { +const Repositories: FC = ({ + namespaces, + repositories, + hasTopExtension, + search, + page, + error, + isLoading, +}) => { const [t] = useTranslation("repos"); - if (namespaces && repositories) { + let header; + if (hasTopExtension) { + header = ( +
+ {t("overview.title")} +
+ ); + } + if (error) { + return ( + <> + {header} + + + ); + } else if (isLoading) { + return ( + <> + {header} + + + ); + } else if (namespaces && repositories) { if (repositories._embedded && repositories._embedded.repositories.length > 0) { return ( <> - + ); } else { - return {t("overview.noRepositories")}; + return ( + <> + {header} + {t("overview.noRepositories")} + + ); } } else { - return {t("overview.invalidNamespace")}; + return ( + <> + {header} + {t("overview.invalidNamespace")} + + ); } }; @@ -146,8 +186,6 @@ const Overview: FC = () => { }; }, [namespace, context]); - const extensions = binder.getExtensions("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 ( { {t("overview.subtitle")} } - loading={isLoading} - error={error} >
- {hasExtensions ? ( + {binder.hasExtension("repository.overview.left") ? ( - {extensions.map((extension) => React.createElement(extension))} + { name="repository.overview.left" renderAll />} ) : null}
- + + + + name="repository.overview.top" + renderAll={true} + props={{ + page, + search, + namespace, + }} + /> + + ("repository.overview.top", { + page, + search, + namespace, + })} + /> + {showCreateButton ? : null}