H-3545, H-4032: Update readonly array display in editor, fix scroll b…

…ehavior in editor slide (#6377)
hashintel · Feb 7, 2025 · 0ba2f48 · 0ba2f48
1 parent e04f6b3
commit 0ba2f48
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 29 deletions.
diff --git a/apps/hash-frontend/src/components/grid/utils/override-custom-renderers.tsx b/apps/hash-frontend/src/components/grid/utils/override-custom-renderers.tsx
@@ -1,11 +1,25 @@
 import type { DataEditorProps } from "@glideapps/glide-data-grid";
 import { isObjectEditorCallbackResult } from "@glideapps/glide-data-grid";
-import type { ReactNode, RefObject } from "react";
+import { type ReactNode, type RefObject } from "react";
 
 import { useEditBarContext } from "../../../shared/edit-bar-scroller";
 import { useScrollLock } from "../../../shared/use-scroll-lock";
 import { InteractableManager } from "./interactable-manager";
 
+/**
+ * Lock scrolling outside when a Grid editor overlay is open.
+ *
+ * Note that because the EditBarContext is set at the Layout level, it passes the `body` as the scrollable component that should be locked.
+ * This doesn't apply when a grid editor is open in a drawer/slide with scroll, which _won't_ have its scroll locked.
+ *
+ * Fixing this requires being able to lock _both_ the body and the slide scroll, which means getting the slide element into this component somehow.
+ * Or having some global context tracking which slide is open, or finding it via classes.
+ *
+ * The type editor slide stack is also relying on useScrollLock to lock the body.
+ * The entity editor slide stack doesn't.
+ *
+ * @todo make the slide stacks consistent when this becomes an issue, and lock the slide scroll.
+ */
 const ScrollLockWrapper = ({
   children,
 }: { children: ReactNode | Promise<ReactNode> }) => {

diff --git a/apps/hash-frontend/src/pages/@/[shortname].page/pinned-entity-type-tab-contents.tsx b/apps/hash-frontend/src/pages/@/[shortname].page/pinned-entity-type-tab-contents.tsx
@@ -197,7 +197,7 @@ export const PinnedEntityTypeTabContents: FunctionComponent<{
     <Box mb={6}>
       <Box display="flex" alignItems="center" columnGap={1.5} marginBottom={1}>
         <ProfileSectionHeading>{currentTab.pluralTitle}</ProfileSectionHeading>
-        <Box display="flex" alignItems="center" columnGap={1}>
+        <Box display="flex" alignItems="center" columnGap={0.5}>
           <Typography
             sx={{
               fontSize: 12,
@@ -222,7 +222,6 @@ export const PinnedEntityTypeTabContents: FunctionComponent<{
                 fontSize: 12,
                 marginRight: 0.5,
                 position: "relative",
-                top: -1,
               },
             }}
             value={sortOrder}

diff --git a/...s/[entity-uuid].page/entity-editor/properties-section/property-table/cells/value-cell.tsx b/...s/[entity-uuid].page/entity-editor/properties-section/property-table/cells/value-cell.tsx
@@ -157,7 +157,7 @@ export const renderValueCell: CustomRenderer<ValueCell> = {
     return undefined;
   },
   provideEditor: ({ data }) => {
-    if (data.readonly) {
+    if (data.readonly && !data.propertyRow.isArray) {
       return {
         disableStyling: true,
         disablePadding: true,

diff --git a/...d].page/entity-editor/properties-section/property-table/cells/value-cell/array-editor.tsx b/...d].page/entity-editor/properties-section/property-table/cells/value-cell/array-editor.tsx
@@ -52,6 +52,9 @@ export const ArrayEditor: ValueCellEditorComponent = ({
   onChange,
 }) => {
   const listWrapperRef = useRef<HTMLDivElement>(null);
+
+  const { readonly } = cell.data;
+
   const {
     value: propertyValue,
     valueMetadata,
@@ -291,7 +294,8 @@ export const ArrayEditor: ValueCellEditorComponent = ({
     updateItem(index, value);
   };
 
-  const canAddMore = isNumber(maxItems) ? items.length < maxItems : true;
+  const canAddMore =
+    !readonly && (isNumber(maxItems) ? items.length < maxItems : true);
   const isAddingDraft = editingRow === DRAFT_ROW_KEY;
 
   const hasConstraints = minItems !== undefined || maxItems !== undefined;
@@ -308,9 +312,10 @@ export const ArrayEditor: ValueCellEditorComponent = ({
           onDragEnd={handleDragEnd}
         >
           <SortableContext items={items} strategy={verticalListSortingStrategy}>
-            {items.map((item) => (
+            {items.map((item, index) => (
               <SortableRow
                 key={item.id}
+                isLastRow={index === items.length - 1}
                 item={item}
                 onRemove={removeItem}
                 onEditClicked={(id) => setEditingRow(id)}
@@ -320,6 +325,7 @@ export const ArrayEditor: ValueCellEditorComponent = ({
                 selected={selectedRow === item.id}
                 onSelect={toggleSelectedRow}
                 expectedTypes={permittedDataTypes}
+                readonly={readonly}
               />
             ))}
           </SortableContext>
@@ -343,7 +349,7 @@ export const ArrayEditor: ValueCellEditorComponent = ({
         />
       )}
 
-      {!canAddMore && hasConstraints && (
+      {!canAddMore && !readonly && hasConstraints && (
         <ItemLimitInfo min={minItems} max={maxItems} />
       )}
     </GridEditorWrapper>

diff --git a/...tity-editor/properties-section/property-table/cells/value-cell/array-editor/draft-row.tsx b/...tity-editor/properties-section/property-table/cells/value-cell/array-editor/draft-row.tsx
@@ -82,6 +82,8 @@ export const DraftRow = ({
         }}
         onDiscardChanges={onDraftDiscarded}
         expectedTypes={expectedTypes}
+        readonly={false}
+        isLastRow
       />
       {arrayConstraints && (
         <ItemLimitInfo

diff --git a/...y-editor/properties-section/property-table/cells/value-cell/array-editor/sortable-row.tsx b/...y-editor/properties-section/property-table/cells/value-cell/array-editor/sortable-row.tsx
@@ -32,11 +32,14 @@ interface SortableRowProps {
   onEditClicked?: (id: string) => void;
   editing: boolean;
   expectedTypes: ClosedDataTypeDefinition[];
+  isLastRow: boolean;
   onSaveChanges: (index: number, value: unknown) => void;
   onDiscardChanges: () => void;
+  readonly: boolean;
 }
 
 export const SortableRow = ({
+  isLastRow,
   item,
   onRemove,
   selected,
@@ -45,6 +48,7 @@ export const SortableRow = ({
   onSaveChanges,
   onDiscardChanges,
   editing,
+  readonly,
 }: SortableRowProps) => {
   const { id, value, index, dataType } = item;
   const {
@@ -80,7 +84,7 @@ export const SortableRow = ({
   const { arrayEditException } = editorSpec;
 
   const shouldShowActions =
-    !isDragging && !isSorting && (hovered || selected || editing);
+    !readonly && !isDragging && !isSorting && (hovered || selected || editing);
 
   if (prevEditing !== editing) {
     setPrevEditing(editing);
@@ -177,7 +181,7 @@ export const SortableRow = ({
         minHeight: 48,
         display: "flex",
         alignItems: "center",
-        borderBottom: "1px solid",
+        borderBottom: isLastRow ? "none" : "1px solid",
         borderColor: isDragging ? "transparent" : "gray.20",
         position: "relative",
         outline: "none",
@@ -186,23 +190,26 @@ export const SortableRow = ({
       onMouseLeave={() => setHovered(false)}
       onClick={() => onSelect?.(id)}
     >
-      <Box
-        {...listeners}
-        sx={{
-          cursor: isDragging || isSorting ? "grabbing" : "grab",
-          px: 1.5,
-          height: "100%",
-          display: "flex",
-          alignItems: "center",
-        }}
-      >
-        <DragIndicatorIcon sx={{ fontSize: 14, color: "gray.50" }} />
-      </Box>
+      {!readonly && (
+        <Box
+          {...listeners}
+          sx={{
+            cursor: isDragging || isSorting ? "grabbing" : "grab",
+            pl: 1.5,
+            height: "100%",
+            display: "flex",
+            alignItems: "center",
+          }}
+        >
+          <DragIndicatorIcon sx={{ fontSize: 14, color: "gray.50" }} />
+        </Box>
+      )}
 
       <Typography
         variant="smallTextLabels"
         sx={{
           color: "gray.50",
+          ml: 1.5,
           mr: 1,
         }}
       >

diff --git a/apps/hash-frontend/src/pages/actions.page.tsx b/apps/hash-frontend/src/pages/actions.page.tsx
@@ -223,10 +223,9 @@ const ActionsPage = () => {
             <Box
               display="flex"
               alignItems="center"
-              columnGap={1}
+              columnGap={0.5}
               sx={{
                 "> div": {
-                  lineHeight: 0,
                   [`.${selectClasses.select}.${inputBaseClasses.input}`]: {
                     fontSize: 14,
                     height: 14,
@@ -239,6 +238,7 @@ const ActionsPage = () => {
                   fontSize: 14,
                   fontWeight: 500,
                   color: ({ palette }) => palette.gray[70],
+                  lineHeight: 1,
                 }}
               >
                 <BarsSortRegularIcon

diff --git a/apps/hash-frontend/src/pages/shared/inline-select.tsx b/apps/hash-frontend/src/pages/shared/inline-select.tsx
@@ -24,19 +24,20 @@ export const InlineSelect = styled(
   ),
 )(({ theme }) => ({
   position: "relative",
-  top: 1,
+  top: 0.5,
   svg: {
     fontSize: 12,
     marginRight: 0.5,
     position: "relative",
-    top: -1,
+    top: -0.5,
   },
   [`.${selectClasses.select}.${inputBaseClasses.input}`]: {
     fontSize: 12,
     height: 12,
     fontWeight: 500,
     color: theme.palette.gray[90],
     minHeight: "unset",
+    padding: 0,
     paddingRight: 18,
     "&:focus": {
       background: "transparent",

diff --git a/apps/hash-frontend/src/shared/use-scroll-lock.ts b/apps/hash-frontend/src/shared/use-scroll-lock.ts
@@ -60,14 +60,23 @@ export const useScrollLock = (
 ) => {
   const scrollbarSize = useLastScrollbarSize(elementToLock);
 
+  const madeChangesRequiringRemoval = useRef(false);
+
   useLayoutEffect(() => {
-    if (active && scrollbarSize) {
+    const overflow = elementToLock.style.overflow;
+
+    const overflowWasAlreadyHidden = overflow === "hidden";
+
+    if (active && scrollbarSize && !overflowWasAlreadyHidden) {
       elementToLock.style.setProperty("padding-right", `${scrollbarSize}px`);
       elementToLock.style.setProperty("overflow", `hidden`);
-    } else {
-      removeStylesFromElement(elementToLock);
+      madeChangesRequiringRemoval.current = true;
     }
 
-    return () => removeStylesFromElement(elementToLock);
+    return () => {
+      if (madeChangesRequiringRemoval.current) {
+        removeStylesFromElement(elementToLock);
+      }
+    };
   }, [active, scrollbarSize, elementToLock]);
 };