chore: code review tweaks

v3/src/components/case-table/case-table-model.ts

@@ -11,9 +11,10 @@ export const CaseTableModel = TileContentModel
     type: types.optional(types.literal(kCaseTableTileType), kCaseTableTileType),
     // key is attribute id; value is width
     columnWidths: types.map(types.number),
+    // key is collection id, value is row height for collection
+    rowHeights: types.map(types.number),
     // Only used for serialization; volatile property used during run time
-    horizontalScrollOffset: 0,
-    rowHeightForCollection: types.map(types.number)
+    horizontalScrollOffset: 0
   })
   .volatile(self => ({
     // entire hierarchical table scrolls as a unit horizontally
@@ -37,6 +38,9 @@ export const CaseTableModel = TileContentModel
     columnWidth(attrId: string) {
       return self.columnWidths.get(attrId)
     },
+    getRowHeightForCollection(collectionId: string) {
+      return self.rowHeights.get(collectionId)
+    }
   }))
   .views(self => {
     const collectionTableModels = new Map<string, CollectionTableModel>()
@@ -45,7 +49,8 @@ export const CaseTableModel = TileContentModel
       getCollectionTableModel(collectionId: string) {
         let collectionTableModel = collectionTableModels.get(collectionId)
         if (!collectionTableModel) {
-          collectionTableModel = new CollectionTableModel(collectionId)
+          const rowHeight = self.getRowHeightForCollection(collectionId)
+          collectionTableModel = new CollectionTableModel(collectionId, rowHeight)
           collectionTableModels.set(collectionId, collectionTableModel)
         }
         return collectionTableModel
@@ -65,7 +70,7 @@ export const CaseTableModel = TileContentModel
       self.columnWidths.replace(columnWidths)
     },
     setRowHeightForCollection(collectionId: string, height: number) {
-      self.rowHeightForCollection.set(collectionId, height)
+      self.rowHeights.set(collectionId, height)
     },
     updateAfterSharedModelChanges(sharedModel?: ISharedModel) {
       // TODO
@@ -74,11 +79,6 @@ export const CaseTableModel = TileContentModel
       self._horizontalScrollOffset = horizontalScrollOffset
     }
   }))
-  .actions(self => ({
-    getRowHeightForCollection(collectionId: string) {
-      return self.rowHeightForCollection.get(collectionId)
-    }
-  }))
 export interface ICaseTableModel extends Instance<typeof CaseTableModel> {}
 export interface ICaseTableSnapshot extends SnapshotIn<typeof CaseTableModel> {}
 

v3/src/components/case-table/case-table-types.ts

@@ -2,6 +2,7 @@ import {
   CalculatedColumn, CellClickArgs, CellKeyDownArgs, CellSelectArgs, ColSpanArgs, Column, RenderCellProps,
   RenderEditCellProps, Renderers, RenderHeaderCellProps, RenderRowProps, RowsChangeData
 } from "react-data-grid"
+import { DEBUG_CASE_IDS } from "../../lib/debug"
 import { IGroupedCase, symFirstChild } from "../../models/data/data-set-types"
 
 export const kCaseTableIdBase = "case-table"
@@ -55,7 +56,7 @@ export type OnScrollRowRangeIntoViewFn = (collectionId: string, rowIndices: numb
 export const kInputRowKey = "__input__"
 
 export const kDropzoneWidth = 30
-export const kIndexColumnWidth = 52
+export const kIndexColumnWidth = DEBUG_CASE_IDS ? 150 : 52
 export const kDefaultColumnWidth = 80
 export const kMinAutoColumnWidth = 50
 export const kMaxAutoColumnWidth = 600
@@ -67,4 +68,7 @@ export const kCaseTableHeaderFont = `bold ${kCaseTableFontSize} ${kCaseTableFont
 export const kCaseTableBodyFont = `${kCaseTableFontSize} ${kCaseTableFontFamily}`
 
 export const kCaseTableDefaultWidth = 580
+export const kDefaultRowHeaderHeight = 30
 export const kDefaultRowHeight = 18
+// used for row resizing
+export const kSnapToLineHeight = 14

v3/src/components/case-table/case-table.scss

@@ -335,6 +335,7 @@ $table-body-font-size: 8pt;
         overflow-wrap: break-word;
 
         .cell-span, .rdg-text-editor {
+          // https://css-tricks.com/almanac/properties/l/line-clamp/
           line-height: 14.5px;
           overflow: hidden;
           text-overflow: ellipsis;
@@ -418,7 +419,6 @@ $table-body-font-size: 8pt;
   .codap-row-divider {
     height: 9px;
     position: absolute;
-    width: 52px;
     cursor: row-resize;
     z-index: 1;
   }

v3/src/components/case-table/collection-table-model.ts

@@ -2,9 +2,8 @@ import { action, computed, makeObservable, observable } from "mobx"
 import { symParent } from "../../models/data/data-set-types"
 import { getNumericCssVariable } from "../../utilities/css-utils"
 import { uniqueId } from "../../utilities/js-utils"
-import { IScrollOptions, kDefaultRowHeight, TRow } from "./case-table-types"
+import { IScrollOptions, kDefaultRowHeaderHeight, kDefaultRowHeight, TRow } from "./case-table-types"
 
-export const kDefaultRowHeaderHeight = 30
 const kDefaultRowCount = 12
 const kDefaultGridHeight = kDefaultRowHeaderHeight + (kDefaultRowCount * kDefaultRowHeight)
 
@@ -46,10 +45,11 @@ export class CollectionTableModel {
   // rows are passed directly to RDG for rendering
   @observable.shallow rows: TRow[] = []
 
-  @observable rowHeight = kDefaultRowHeight
+  @observable rowHeight
 
-  constructor(collectionId: string) {
+  constructor(collectionId: string, rowHeight = kDefaultRowHeight) {
     this.collectionId = collectionId
+    this.rowHeight = rowHeight
 
     makeObservable(this)
   }
@@ -62,7 +62,6 @@ export class CollectionTableModel {
     return getNumericCssVariable(this.element, "--rdg-row-header-height") ?? kDefaultRowHeaderHeight
   }
 
-
   // visible height of the body of the grid, i.e. the rows excluding the header row
   get gridBodyHeight() {
     return (this.element?.getBoundingClientRect().height ?? kDefaultGridHeight) - kDefaultRowHeaderHeight
@@ -72,12 +71,8 @@ export class CollectionTableModel {
     return rowIndex * this.rowHeight
   }
 
-  @computed get rowBottoms() {
-    return this.rows.map((_, rowIndex) => (rowIndex + 1) * this.rowHeight)
-  }
-
   getRowBottom(rowIndex: number) {
-    return this.rowBottoms[rowIndex]
+    return (rowIndex + 1) * this.rowHeight
   }
 
   get isSyncScrollingResponseDisabled() {

v3/src/components/case-table/collection-table.tsx

@@ -4,8 +4,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"
 import DataGrid, { CellKeyboardEvent, DataGridHandle } from "react-data-grid"
 import { kCollectionTableBodyDropZoneBaseId } from "./case-table-drag-drop"
 import {
-  kDefaultRowHeight,
-  kInputRowKey, OnScrollClosestRowIntoViewFn, OnScrollRowRangeIntoViewFn, OnTableScrollFn,
+  kDefaultRowHeight, kInputRowKey, OnScrollClosestRowIntoViewFn, OnScrollRowRangeIntoViewFn, OnTableScrollFn,
   TCellKeyDownArgs, TRenderers, TRow
 } from "./case-table-types"
 import { CollectionTableSpacer } from "./collection-table-spacer"
@@ -76,13 +75,8 @@ export const CollectionTable = observer(function CollectionTable(props: IProps)
   const [selectionStartRowIdx, setSelectionStartRowIdx] = useState<number | null>(null)
   const initialPointerDownPosition = useRef({ x: 0, y: 0 })
   const kPointerMovementThreshold = 3
-  const rowHeight = caseTableModel?.getRowHeightForCollection(collectionId) ??
-                    collectionTableModel?.rowHeight ??
-                    kDefaultRowHeight
+  const rowHeight = collectionTableModel?.rowHeight ?? kDefaultRowHeight
 
-  if (caseTableModel?.getRowHeightForCollection(collectionId)  !== collectionTableModel?.rowHeight) {
-    collectionTableModel?.setRowHeight(rowHeight)
-  }
   useEffect(function setGridElement() {
     const element = gridRef.current?.element
     if (element && collectionTableModel) {
@@ -100,7 +94,7 @@ export const CollectionTable = observer(function CollectionTable(props: IProps)
 
   // columns
   const indexColumn = useIndexColumn()
-  const columns = useColumns({ data, indexColumn, rowHeight })
+  const columns = useColumns({ data, indexColumn })
 
   // rows
   const { handleRowsChange } = useRows(gridRef.current?.element ?? null)

v3/src/components/case-table/row-divider.tsx

@@ -1,16 +1,14 @@
 import { clsx } from "clsx"
+import { observer } from "mobx-react-lite"
 import React, { useEffect, useRef, useState } from "react"
 import { createPortal } from "react-dom"
 import { useCollectionContext } from "../../hooks/use-collection-context"
-import { useCollectionTableModel } from "./use-collection-table-model"
-import { kDefaultRowHeight } from "./case-table-types"
+import { kDefaultRowHeaderHeight, kDefaultRowHeight, kIndexColumnWidth, kSnapToLineHeight } from "./case-table-types"
 import { useCaseTableModel } from "./use-case-table-model"
-import { observer } from "mobx-react-lite"
-import { kDefaultRowHeaderHeight } from "./collection-table-model"
+import { useCollectionTableModel } from "./use-collection-table-model"
 
 const kTableRowDividerHeight = 13
 const kTableDividerOffset = Math.floor(kTableRowDividerHeight / 2)
-export const kSnapToLineHeight = 14
 interface IRowDividerProps {
   rowId: string
 }
@@ -20,37 +18,29 @@ export const RowDivider = observer(function RowDivider({ rowId }: IRowDividerPro
   const collectionTable = collectionTableModel?.element
   const caseTableModel = useCaseTableModel()
   const rows = collectionTableModel?.rows
-  const rowHeightRef = useRef(kDefaultRowHeight)
   const isResizing = useRef(false)
   const startY = useRef(0)
-  const initialHeight = useRef(rowHeightRef.current)
+  const getRowHeight = () => collectionTableModel?.rowHeight ?? kDefaultRowHeight
+  const initialHeight = useRef(getRowHeight())
   const rowIdx = rows?.findIndex(row => row.__id__ === rowId)
   const [rowElement, setRowElement] = useState<HTMLDivElement | null>(null)
   const getRowBottom = () => {
-    if (rowIdx === 0) return collectionTableModel?.rowHeight ?? kDefaultRowHeight
+    if (rowIdx === 0) return getRowHeight()
     else return (rowIdx && collectionTableModel?.getRowBottom(rowIdx)) ?? kDefaultRowHeight
   }
 
+
   useEffect(() => {
     (rowIdx != null && collectionTable) &&
       setRowElement(collectionTable.querySelector(`[aria-rowindex="${rowIdx + 2}"]`) as HTMLDivElement)
   }, [collectionTable, rowIdx])
 
-  useEffect(() => {
-    if (!caseTableModel?.getRowHeightForCollection(collectionId)) {
-      caseTableModel?.setRowHeightForCollection(collectionId, rowHeightRef.current)
-      collectionTableModel?.setRowHeight(rowHeightRef.current)
-    }
-  }, [caseTableModel, collectionId, collectionTableModel])
-
   // allow the user to drag the divider
   const handleMouseDown = (e: React.MouseEvent) => {
     e.stopPropagation()
     isResizing.current = true
     startY.current = e.clientY
-    initialHeight.current = collectionTableModel?.rowHeight ??
-                            caseTableModel?.getRowHeightForCollection(collectionId) ??
-                            kDefaultRowHeight
+    initialHeight.current = getRowHeight()
     document.addEventListener("mousemove", handleMouseMove)
     document.addEventListener("mouseup", handleMouseUp)
   }
@@ -61,18 +51,15 @@ export const RowDivider = observer(function RowDivider({ rowId }: IRowDividerPro
       const deltaY = e.clientY - startY.current
       const tempNewHeight = Math.max(kDefaultRowHeight, initialHeight.current + deltaY)
       const newHeight = kSnapToLineHeight * Math.round((tempNewHeight - 4) / kSnapToLineHeight) + 4
-      rowHeightRef.current = newHeight
       collectionTableModel?.setRowHeight(newHeight)
     }
   }
 
   const handleMouseUp = (e: MouseEvent) => {
     e.stopPropagation()
     isResizing.current = false
-    initialHeight.current = rowHeightRef.current
-    collectionTableModel?.setRowHeight(rowHeightRef.current)
     caseTableModel?.applyModelChange(() => {
-        caseTableModel?.setRowHeightForCollection(collectionId, rowHeightRef.current)
+        caseTableModel?.setRowHeightForCollection(collectionId, getRowHeight())
       },
       {
         log: "Change row height",
@@ -85,12 +72,13 @@ export const RowDivider = observer(function RowDivider({ rowId }: IRowDividerPro
   }
 
   const className = clsx("codap-row-divider")
+  const top = getRowBottom() + kDefaultRowHeaderHeight - kTableDividerOffset
+  const width = kIndexColumnWidth
   return (
     rowElement
       ? createPortal((
             <div className={className} onMouseDown={handleMouseDown}
-                  data-testid={`row-divider-${rowIdx}`}
-                  style={{top: getRowBottom() + kDefaultRowHeaderHeight - kTableDividerOffset}}
+                  data-testid={`row-divider-${rowIdx}`} style={{top, width}}
             />
           ), rowElement)
       : null

v3/src/components/case-table/use-columns.tsx

@@ -13,17 +13,19 @@ import { kDefaultColumnWidth, kDefaultRowHeight, TColumn } from "./case-table-ty
 import CellTextEditor from "./cell-text-editor"
 import ColorCellTextEditor from "./color-cell-text-editor"
 import { ColumnHeader } from "./column-header"
+import { useCollectionTableModel } from "./use-collection-table-model"
 
 interface IUseColumnsProps {
   data?: IDataSet
   indexColumn?: TColumn
-  rowHeight: number
 }
-export const useColumns = ({ data, indexColumn, rowHeight }: IUseColumnsProps) => {
+export const useColumns = ({ data, indexColumn }: IUseColumnsProps) => {
   const caseMetadata = useCaseMetadata()
+  const collectionTableModel = useCollectionTableModel()
   const parentCollection = useParentCollectionContext()
   const collectionId = useCollectionContext()
   const [columns, setColumns] = useState<TColumn[]>([])
+  const rowHeight = collectionTableModel?.rowHeight ?? kDefaultRowHeight
 
   useEffect(() => {
     // rebuild column definitions when referenced properties change

v3/src/components/case-table/use-index-column.tsx

@@ -16,14 +16,12 @@ import { t } from "../../utilities/translation/translate"
 import { kIndexColumnKey } from "../case-tile-common/case-tile-types"
 import { IndexMenuList } from "../case-tile-common/index-menu-list"
 import { useParentChildFocusRedirect } from "../case-tile-common/use-parent-child-focus-redirect"
-import { kInputRowKey, TColSpanArgs, TColumn, TRenderCellProps } from "./case-table-types"
+import { kIndexColumnWidth, kInputRowKey, TColSpanArgs, TColumn, TRenderCellProps } from "./case-table-types"
 import { ColumnHeader } from "./column-header"
 import { RowDivider } from "./row-divider"
 
 import DragIndicator from "../../assets/icons/drag-indicator.svg"
 
-const kIndexColumnWidth = DEBUG_CASE_IDS ? 150 : 52
-
 interface IColSpanProps {
   data?: IDataSet
   metadata?: ISharedCaseMetadata
@@ -76,7 +74,7 @@ export const useIndexColumn = () => {
       <div className="codap-index-cell-wrapper">
         <IndexCell caseId={__id__} disableMenu={disableMenu} index={index}
         collapsedCases={collapsedCaseCount} onClick={handleClick} />
-        {(!isInputRow) && <RowDivider rowId={row.__id__} />}
+        {(!isInputRow) && <RowDivider rowId={row.__id__}/>}
       </div>
     )
   }, [caseMetadata, data, disableMenu])

v3/src/components/case-table/use-rows.ts

@@ -93,7 +93,7 @@ export const useRows = (gridElement: HTMLDivElement | null) => {
         })
       })
     })
-  }, [collectionId, data, gridElement, setCachedDomAttr])
+  }, [collectionId, collectionTableModel, data, gridElement, setCachedDomAttr])
 
   const resetRowCacheAndSyncRows = useDebouncedCallback(() => {
     resetRowCache()

v3/src/components/case-tile-common/attribute-format-utils.tsx

@@ -9,9 +9,10 @@ import { parseColor } from "../../utilities/color-utils"
 import { isStdISODateString } from "../../utilities/date-iso-utils"
 import { parseDate } from "../../utilities/date-parser"
 import { formatDate } from "../../utilities/date-utils"
-import { kCaseTableBodyFont, kCaseTableHeaderFont, kDefaultRowHeight, kMaxAutoColumnWidth,
-          kMinAutoColumnWidth } from "../case-table/case-table-types"
-import { kSnapToLineHeight } from "../case-table/row-divider"
+import {
+  kCaseTableBodyFont, kCaseTableHeaderFont, kDefaultRowHeight,
+  kMaxAutoColumnWidth, kMinAutoColumnWidth, kSnapToLineHeight
+} from "../case-table/case-table-types"
 
 // cache d3 number formatters so we don't have to generate them on every render
 type TNumberFormatter = (n: number) => string
@@ -30,9 +31,9 @@ export function renderAttributeValue(str = "", num = NaN, showUnits = false, att
             key?: number, rowHeight: number = kDefaultRowHeight) {
   const { type, userType, numPrecision, datePrecision } = attr || {}
   let formatClass = ""
-  // [CC] https://css-tricks.com/almanac/properties/l/line-clamp/
+  // https://css-tricks.com/almanac/properties/l/line-clamp/
   const lineClamp = rowHeight > kDefaultRowHeight
-                      ? Math.ceil((rowHeight) / (kSnapToLineHeight + 1))
+                      ? Math.ceil(rowHeight / (kSnapToLineHeight + 1))
                       : 0
 
   // boundaries