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>

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);
     }),

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() {

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 {

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) {

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,

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) => {
-          renderTableContents(item).map((content, index) => {
+                  const cellProps: TableCellProps = isReactNode(content)
-            const cellProps: TableCellProps = isReactNode(content) ? { children: content } : 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}
+              onRemove={
-          removeTooltip={`Remove selected items (${selectedItems.length})`}
+                store.selectedItems.length ? this.removeItemsDialog : null
+              }
+              removeTooltip={`Remove selected items (${store.selectedItems.length})`}
               {...addRemoveButtons}
             />
+          )}
+        </Observer>
       </div>
     );
   }

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);