#690 Fix Resource cell keyboard navigation, #675 Don't notify when la…

…st-commit has not changed

browser/data-browser/src/components/TableEditor/Cell.tsx

@@ -1,4 +1,10 @@
-import React, { useCallback, useEffect, useLayoutEffect, useRef } from 'react';
+import React, {
+  useCallback,
+  useEffect,
+  useLayoutEffect,
+  useRef,
+  useState,
+} from 'react';
 import { styled } from 'styled-components';
 import {
   CursorMode,
@@ -44,6 +50,8 @@ export function Cell({
 }: React.PropsWithChildren<CellProps>): JSX.Element {
   const ref = useRef<HTMLDivElement>(null);
 
+  const [markEnterEditMode, setMarkEnterEditMode] = useState(false);
+
   const {
     mouseDown,
     selectedRow,
@@ -77,6 +85,20 @@ export function Cell({
     }
   }, [mouseDown, rowIndex, columnIndex]);
 
+  const shouldEnterEditMode = useCallback(
+    (e: React.MouseEvent<HTMLDivElement>) => {
+      // @ts-ignore
+      if (e.target.tagName === 'INPUT' || e.target.tagName === 'BUTTON') {
+        // If the user clicked on an input don't enter edit mode. (Necessary for normal checkbox behavior)
+        return false;
+      }
+
+      // Enter edit mode when clicking on a higlighted cell, except when it's the index column.
+      return isActive && columnIndex !== 0;
+    },
+    [isActive, columnIndex],
+  );
+
   const handleMouseDown = useCallback(
     (e: React.MouseEvent<HTMLDivElement>) => {
       setMouseDown(true);
@@ -97,18 +119,10 @@ export function Cell({
         return;
       }
 
-      // @ts-ignore
-      if (e.target.tagName === 'INPUT' || e.target.tagName === 'BUTTON') {
-        // If the user clicked on an input don't enter edit mode. (Necessary for normal checkbox behavior)
-        return;
-      }
+      if (shouldEnterEditMode(e)) {
+        setMarkEnterEditMode(true);
 
-      if (isActive && columnIndex !== 0) {
-        // Enter edit mode when clicking on a higlighted cell, except when it's the index column.
-        setMultiSelectCorner(undefined, undefined);
-        setMouseDown(false);
-
-        return setCursorMode(CursorMode.Edit);
+        return;
       }
 
       if (disabledKeyboardInteractions.has(KeyboardInteraction.ExitEditMode)) {
@@ -118,9 +132,19 @@ export function Cell({
       setCursorMode(CursorMode.Visual);
       setActiveCell(rowIndex, columnIndex);
     },
-    [setActiveCell, isActive, columnIndex, disabledKeyboardInteractions],
+    [setActiveCell, columnIndex, shouldEnterEditMode],
   );
 
+  const handleClick = useCallback(() => {
+    if (markEnterEditMode) {
+      setMultiSelectCorner(undefined, undefined);
+      setMouseDown(false);
+
+      setCursorMode(CursorMode.Edit);
+      setMarkEnterEditMode(false);
+    }
+  }, [markEnterEditMode]);
+
   useLayoutEffect(() => {
     if (!ref.current) {
       return;
@@ -178,6 +202,7 @@ export function Cell({
       tabIndex={isActive ? 0 : -1}
       onMouseDown={handleMouseDown}
       onMouseUp={handleMouseUp}
+      onClick={handleClick}
       onMouseEnter={handleMouseEnter}
     >
       {children}

browser/data-browser/src/components/TableEditor/hooks/useCellSizes.ts

@@ -58,6 +58,10 @@ export function useCellSizes<T>(
   }, [externalSizes]);
 
   useEffect(() => {
+    if (columns.length === 0) {
+      return;
+    }
+
     const diff = columns.length - sizes.length;
 
     if (diff > 0) {

browser/data-browser/src/views/TablePage/EditorCells/AtomicURLCell.tsx

@@ -1,4 +1,5 @@
 import {
+  Core,
   JSONValue,
   Resource,
   unknownSubject,
@@ -47,36 +48,44 @@ import {
 } from '../../../components/TableEditor';
 
 const useClassType = (subject: string) => {
-  const resource = useResource(subject);
-  const [classType] = useString(resource, urls.properties.classType);
-  const classTypeResource = useResource(classType);
-  const [classTypeTitle] = useTitle(classTypeResource);
+  const property = useResource<Core.Property>(subject);
+
+  const classType = useResource<Core.Class>(property.props.classtype);
+  const hasClassType = classType?.getSubject() !== unknownSubject;
 
   return {
     classType,
-    classTypeTitle,
+    hasClassType,
   };
 };
 
-const cellOptions = {
-  disabledKeyboardInteractions: new Set([KeyboardInteraction.EditNextRow]),
-};
-
 function AtomicURLCellEdit({
   value,
   onChange,
   property,
   resource: row,
 }: EditCellProps<JSONValue>): JSX.Element {
   const cell = useResource(value as string);
-  const { classType, classTypeTitle } = useClassType(property);
+  const { classType, hasClassType } = useClassType(property);
   const [title] = useTitle(cell);
   const [open, setOpen] = useState(true);
   const { setCursorMode } = useTableEditorContext();
   const selectedElement = useRef<HTMLLIElement>(null);
 
   const [searchValue, setSearchValue] = useState('');
 
+  const cellOptions = useMemo(() => {
+    if (open) {
+      return {
+        disabledKeyboardInteractions: new Set([
+          KeyboardInteraction.ExitEditMode,
+        ]),
+      };
+    } else {
+      return {};
+    }
+  }, [open]);
+
   useCellOptions(cellOptions);
 
   const handleChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
@@ -106,7 +115,7 @@ function AtomicURLCellEdit({
 
   const { results, selectedIndex, handleKeyDown } = useResourceSearch(
     searchValue,
-    classType,
+    hasClassType ? classType.getSubject() : undefined,
     handleResultClick,
   );
 
@@ -119,6 +128,10 @@ function AtomicURLCellEdit({
         return;
       }
 
+      if (e.key === 'Tab') {
+        return;
+      }
+
       handleKeyDown(e);
     },
     [handleKeyDown],
@@ -141,29 +154,31 @@ function AtomicURLCellEdit({
       <PopoverTrigger>
         <FaEdit />{' '}
         {cell.getSubject() === unknownSubject
-          ? `select ${classTypeTitle ?? 'resource'}`
+          ? `select ${hasClassType ? classType.title : 'resource'}`
           : title}
       </PopoverTrigger>
     );
-  }, [title, cell, classTypeTitle]);
+  }, [title, cell, classType, hasClassType]);
 
   useEffect(() => {
     if (selectedElement.current) {
       selectedElement.current.scrollIntoView(false);
     }
   }, [selectedIndex]);
 
-  const placehoder = classType ? `Search ${classTypeTitle}` : 'Search...';
+  const placehoder = hasClassType ? `Search ${classType.title}` : 'Search...';
 
   const showFileDropzone =
-    results.length === 0 && classType === urls.classes.file;
-  const showNoResults = results.length === 0 && classType !== urls.classes.file;
+    results.length === 0 && classType.getSubject() === urls.classes.file;
+  const showNoResults =
+    results.length === 0 && classType.getSubject() !== urls.classes.file;
 
   return (
     <SearchPopover
       Trigger={Trigger}
       open={open}
       onOpenChange={handleOpenChange}
+      noLock
     >
       <InputWrapper>
         <InputStyled

browser/data-browser/src/views/TablePage/TablePage.tsx

@@ -26,14 +26,8 @@ export function TablePage({ resource }: ResourcePageProps): JSX.Element {
   const store = useStore();
   const titleId = useId();
 
-  const {
-    tableClass,
-    sorting,
-    setSortBy,
-    collection,
-    invalidateCollection,
-    collectionVersion,
-  } = useTableData(resource);
+  const { tableClass, sorting, setSortBy, collection, invalidateCollection } =
+    useTableData(resource);
 
   const { columns, reorderColumns } = useTableColumns(tableClass);
 
@@ -46,7 +40,6 @@ export function TablePage({ resource }: ResourcePageProps): JSX.Element {
     tableClass,
     invalidateCollection,
     addItemsToHistoryStack,
-    collectionVersion,
   );
 
   const [showExpandedRowDialog, setShowExpandedRowDialog] = useState(false);
@@ -94,7 +87,7 @@ export function TablePage({ resource }: ResourcePageProps): JSX.Element {
     addItemsToHistoryStack,
   );
 
-  const handleCopyCommand = useHandleCopyCommand(collection, collectionVersion);
+  const handleCopyCommand = useHandleCopyCommand(collection);
 
   const [columnSizes, handleColumnResize] = useHandleColumnResize(resource);
 
@@ -115,13 +108,9 @@ export function TablePage({ resource }: ResourcePageProps): JSX.Element {
         />
       );
     },
-    [collection, columns, collectionVersion],
+    [collection, columns],
   );
 
-  if (collectionVersion === 0) {
-    return <ContainerFull>Loading...</ContainerFull>;
-  }
-
   return (
     <ContainerFull>
       <TablePageContext.Provider value={tablePageContext}>

browser/data-browser/src/views/TablePage/helpers/useHandleCopyCommand.ts

@@ -4,10 +4,7 @@ import { CellIndex, CopyValue } from '../../../components/TableEditor';
 import { getValuesFromSubject } from './clipboard';
 import { transformToPropertiesPerSubject } from './transformPropertiesPerSubject';
 
-export function useHandleCopyCommand(
-  collection: Collection,
-  collectionVersion: number,
-) {
+export function useHandleCopyCommand(collection: Collection) {
   const store = useStore();
 
   return useCallback(
@@ -24,6 +21,6 @@ export function useHandleCopyCommand(
       return Promise.all(unresolvedValues);
     },
 
-    [collection, collectionVersion, store],
+    [collection, store],
   );
 }

browser/data-browser/src/views/TablePage/helpers/useHandlePaste.ts

@@ -21,7 +21,6 @@ export function useHandlePaste(
   tableClass: Resource,
   invalidateCollection: () => void,
   addHistoryItemBatchToStack: (historyItemBatch: HistoryItemBatch) => void,
-  collectionVersion: number,
 ) {
   const store = useStore();
 
@@ -87,6 +86,6 @@ export function useHandlePaste(
         invalidateCollection();
       }
     },
-    [collectionVersion, collection, invalidateCollection, store],
+    [collection, invalidateCollection, store],
   );
 }

browser/lib/src/collection.ts

@@ -22,6 +22,7 @@ export interface CollectionParams extends QueryFilter {
  * Use the `invalidate` method to force a refresh.
  */
 export class Collection {
+  public readonly __internalObject = this;
   private store: Store;
   private pages = new Map<number, Resource>();
   private server: string;
@@ -153,3 +154,11 @@ export class Collection {
     this._totalMembers = totalMembers;
   }
 }
+
+export function proxyCollection(collection: Collection): Collection {
+  if (collection.__internalObject !== collection) {
+    console.warn('Attempted to proxy a proxy for a collection');
+  }
+
+  return new Proxy(collection.__internalObject, {});
+}

browser/lib/src/commit.ts

@@ -489,7 +489,7 @@ export function parseAndApplyCommit(jsonAdObjStr: string, store: Store) {
   } else {
     resource.appliedCommitSignatures.add(signature);
 
-    store.addResources(resource);
+    store.addResources(resource, { skipCommitCompare: true });
   }
 }
 
@@ -522,7 +522,7 @@ function execSetCommit(
     resource.setUnsafe(key, newVal);
   }
 
-  store && store.addResources(...parsedResources);
+  store && store.addResources(parsedResources);
 }
 
 function execRemoveCommit(remove: string[], resource: Resource) {
@@ -556,5 +556,5 @@ function execPushCommit(
     resource.setUnsafe(key, new_arr);
   }
 
-  store && store.addResources(...parsedResources);
+  store && store.addResources(parsedResources);
 }

browser/lib/src/resource.ts

@@ -98,6 +98,10 @@ export class Resource<C extends OptionalClass = any> {
 
   /** Checks if the content of two Resource instances is equal */
   public equals(resourceB: Resource): boolean {
+    if (this === resourceB.__internalObject) {
+      return true;
+    }
+
     if (this.getSubject() !== resourceB.getSubject()) {
       return false;
     }
@@ -115,14 +119,14 @@ export class Resource<C extends OptionalClass = any> {
     }
 
     if (
-      JSON.stringify(Array.from(this.propvals.entries())) ===
+      JSON.stringify(Array.from(this.propvals.entries())) !==
       JSON.stringify(Array.from(resourceB.propvals.entries()))
     ) {
       return false;
     }
 
     if (
-      JSON.stringify(Array.from(this.commitBuilder.set.entries())) ===
+      JSON.stringify(Array.from(this.commitBuilder.set.entries())) !==
       JSON.stringify(Array.from(resourceB.commitBuilder.set.entries()))
     ) {
       return false;
@@ -497,7 +501,7 @@ export class Resource<C extends OptionalClass = any> {
 
     try {
       this.commitError = undefined;
-      store.addResources(this);
+      store.addResources(this, { skipCommitCompare: true });
       const createdCommit = await store.postCommit(commit, endpoint);
       // const res = store.getResourceLoading(this.subject);
       this.setUnsafe(properties.commit.lastCommit, createdCommit.id!);
@@ -542,7 +546,7 @@ export class Resource<C extends OptionalClass = any> {
       // If it fails, revert to the old resource with the old CommitBuilder
       this.commitBuilder = oldCommitBuilder;
       this.commitError = e;
-      store.addResources(this);
+      store.addResources(this, { skipCommitCompare: true });
       throw e;
     }
   }