Fix bugs in ItemListLayouts/KubeObjectListLayouts (#4792)

* Fix virtual list behaviour in item list layout by making table rows observer Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make selecting item in ItemListLayout not scroll to top Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix scrolling to top when selecting all items in item list layout Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix refreshing release values when release is unselected Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Remove noisy debugging Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix sorting of table rows Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Replace avoidable asyncComputed with normal computed Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Revert unnecessary code style changes Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix render-storm in Events Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix missing key in React array Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
2025-05-20 05:10:56 +00:00 · 2022-02-02 15:46:07 +01:00 · 2022-02-02 15:46:07 +01:00 · e58d9b631a
commit e58d9b631a
parent 334ab56f9d
7 changed files with 95 additions and 70 deletions
--- a/src/renderer/components/+apps-releases/release-details/release-details.injectable.ts
+++ b/src/renderer/components/+apps-releases/release-details/release-details.injectable.ts
@ -10,7 +10,7 @@ import releaseInjectable from "./release.injectable";
 const releaseDetailsInjectable = getInjectable({
  instantiate: (di) =>
    asyncComputed(async () => {
-      const release = di.inject(releaseInjectable).value.get();
+      const release = di.inject(releaseInjectable).get();

      return await getRelease(release.name, release.namespace);
    }),
--- a/src/renderer/components/+apps-releases/release-details/release-details.tsx
+++ b/src/renderer/components/+apps-releases/release-details/release-details.tsx
@ -7,7 +7,7 @@ import "./release-details.scss";

 import React, { Component } from "react";
 import groupBy from "lodash/groupBy";
-import { computed, makeObservable, observable } from "mobx";
+import { computed, IComputedValue, makeObservable, observable } from "mobx";
 import { Link } from "react-router-dom";
 import kebabCase from "lodash/kebabCase";
 import type { HelmRelease, IReleaseDetails, IReleaseUpdateDetails, IReleaseUpdatePayload } from "../../../../common/k8s-api/endpoints/helm-releases.api";
@ -39,7 +39,7 @@ interface Props {
 }

 interface Dependencies {
-  release: IAsyncComputed<HelmRelease>
+  release: IComputedValue<HelmRelease>
  releaseDetails: IAsyncComputed<IReleaseDetails>
  releaseValues: IAsyncComputed<string>
  updateRelease: (name: string, namespace: string, payload: IReleaseUpdatePayload) => Promise<IReleaseUpdateDetails>
@ -59,7 +59,7 @@ class NonInjectedReleaseDetails extends Component<Props & Dependencies> {
  }

  @computed get release() {
-    return this.props.release.value.get();
+    return this.props.release.get();
  }

  @computed get details() {
--- a/src/renderer/components/+apps-releases/release-details/release-values.injectable.ts
+++ b/src/renderer/components/+apps-releases/release-details/release-values.injectable.ts
@ -12,7 +12,13 @@ import userSuppliedValuesAreShownInjectable from "./user-supplied-values-are-sho
 const releaseValuesInjectable = getInjectable({
  instantiate: (di) =>
    asyncComputed(async () => {
-      const release = di.inject(releaseInjectable).value.get();
+      const release = di.inject(releaseInjectable).get();
+
+      // TODO: Figure out way to get rid of defensive code
+      if (!release) {
+        return "";
+      }
+
      const userSuppliedValuesAreShown = di.inject(userSuppliedValuesAreShownInjectable).value;

      try {
--- a/src/renderer/components/+apps-releases/release-details/release.injectable.ts
+++ b/src/renderer/components/+apps-releases/release-details/release.injectable.ts
@ -6,14 +6,14 @@ import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
 import { matches } from "lodash/fp";
 import releasesInjectable from "../releases.injectable";
 import releaseRouteParametersInjectable from "./release-route-parameters.injectable";
-import { asyncComputed } from "@ogre-tools/injectable-react";
+import { computed } from "mobx";

 const releaseInjectable = getInjectable({
  instantiate: (di) => {
    const releases = di.inject(releasesInjectable);
    const releaseRouteParameters = di.inject(releaseRouteParametersInjectable);

-    return asyncComputed(async () => {
+    return computed(() => {
      const { name, namespace } = releaseRouteParameters.get();

      if (!name || !namespace) {
--- a/src/renderer/components/+events/events.tsx
+++ b/src/renderer/components/+events/events.tsx
@ -124,7 +124,7 @@ export class Events extends React.Component<Props> {
  };

  render() {
-    const { store, visibleItems } = this;
+    const { store } = this;
    const { compact, compactLimit, className, ...layoutProps } = this.props;

    const events = (
@ -137,7 +137,7 @@ export class Events extends React.Component<Props> {
        renderHeaderTitle="Events"
        customizeHeader={this.customizeHeader}
        isSelectable={false}
-        items={visibleItems}
+        getItems={() => this.visibleItems}
        virtual={!compact}
        tableProps={{
          sortSyncWithUrl: false,
--- a/src/renderer/components/item-object-list/content.tsx
+++ b/src/renderer/components/item-object-list/content.tsx
@ -7,7 +7,7 @@ import "./item-list-layout.scss";

 import React, { ReactNode } from "react";
 import { computed, makeObservable } from "mobx";
-import { observer } from "mobx-react";
+import { Observer, observer } from "mobx-react";
 import { ConfirmDialog, ConfirmDialogParams } from "../confirm-dialog";
 import { Table, TableCell, TableCellProps, TableHead, TableProps, TableRow, TableRowProps, TableSortCallbacks } from "../table";
 import { boundMethod, cssNames, IClassName, isReactNode, prevDefault, stopPropagation } from "../../utils";
@ -70,6 +70,10 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component

  @boundMethod
  getRow(uid: string) {
+    return (
+      <div key={uid}>
+        <Observer>
+          {() => {
            const {
              isSelectable, renderTableHeader, renderTableContents, renderItemMenu,
              store, hasDetailsView, onDetails,
@ -83,7 +87,6 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component

            return (
              <TableRow
-        key={itemId}
                nowrap
                searchItem={item}
                sortItem={item}
@ -98,13 +101,17 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
                    onClick={prevDefault(() => store.toggleSelection(item))}
                  />
                )}
-        {
-          renderTableContents(item).map((content, index) => {
-            const cellProps: TableCellProps = isReactNode(content) ? { children: content } : content;
+                {renderTableContents(item).map((content, index) => {
+                  const cellProps: TableCellProps = isReactNode(content)
+                    ? { children: content }
+                    : content;
                  const headCell = renderTableHeader?.[index];

                  if (copyClassNameFromHeadCells && headCell) {
-              cellProps.className = cssNames(cellProps.className, headCell.className);
+                    cellProps.className = cssNames(
+                      cellProps.className,
+                      headCell.className,
+                    );
                  }

                  if (!headCell || this.showColumn(headCell)) {
@ -112,8 +119,7 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
                  }

                  return null;
-          })
-        }
+                })}
                {renderItemMenu && (
                  <TableCell className="menu">
                    <div onClick={stopPropagation}>
@ -123,6 +129,10 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
                )}
              </TableRow>
            );
+          }}
+        </Observer>
+      </div>
+    );
  }

  @boundMethod
@ -190,12 +200,16 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
    return (
      <TableHead showTopLine nowrap>
        {isSelectable && (
+          <Observer>
+            {() => (
              <TableCell
                checkbox
                isChecked={store.isSelectedAll(enabledItems)}
                onClick={prevDefault(() => store.toggleSelectionAll(enabledItems))}
              />
            )}
+          </Observer>
+        )}
        {renderTableHeader.map((cellProps, index) => (
          this.showColumn(cellProps) && (
            <TableCell key={cellProps.id ?? index} {...cellProps} />
@ -213,7 +227,6 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
      store, hasDetailsView, addRemoveButtons = {}, virtual, sortingCallbacks,
      detailsItem, className, tableProps = {}, tableId,
    } = this.props;
-    const { selectedItems } = store;
    const selectedItemId = detailsItem && detailsItem.getId();
    const classNames = cssNames(className, "box", "grow", ThemeStore.getInstance().activeTheme.type);

@ -234,11 +247,18 @@ export class ItemListLayoutContent<I extends ItemObject> extends React.Component
          {this.renderTableHeader()}
          {this.renderItems()}
        </Table>
+
+        <Observer>
+          {() => (
            <AddRemoveButtons
-          onRemove={selectedItems.length ? this.removeItemsDialog : null}
-          removeTooltip={`Remove selected items (${selectedItems.length})`}
+              onRemove={
+                store.selectedItems.length ? this.removeItemsDialog : null
+              }
+              removeTooltip={`Remove selected items (${store.selectedItems.length})`}
              {...addRemoveButtons}
            />
+          )}
+        </Observer>
      </div>
    );
  }
--- a/src/renderer/components/kube-detail-params/params.ts
+++ b/src/renderer/components/kube-detail-params/params.ts
@ -46,7 +46,6 @@ export function hideDetails() {
 }

 export function getDetailsUrl(selfLink: string, resetSelected = false, mergeGlobals = true) {
-  console.debug("getDetailsUrl", { selfLink, resetSelected, mergeGlobals });
  const params = new URLSearchParams(mergeGlobals ? navigation.searchParams : "");

  params.set(kubeDetailsUrlParam.name, selfLink);