Merge pull request #3037 from mathesar-foundation/selection

Refactor CellSelection data structure and store

mathesar_ui/.eslintrc.cjs

@@ -30,6 +30,7 @@ module.exports = {
   rules: {
     'import/no-extraneous-dependencies': ['error', { devDependencies: true }],
     'no-console': ['warn', { allow: ['error'] }],
+    'generator-star-spacing': 'off',
     'no-continue': 'off',
     '@typescript-eslint/explicit-member-accessibility': 'off',
     '@typescript-eslint/ban-ts-comment': [

mathesar_ui/package.json

@@ -56,7 +56,7 @@
     "dayjs": "^1.11.5",
     "fast-diff": "^1.2.0",
     "flatpickr": "^4.6.13",
-    "iter-tools": "^7.4.0",
+    "iter-tools": "^7.5.3",
     "js-cookie": "^3.0.1",
     "papaparse": "^5.4.1",
     "perfect-scrollbar": "^1.5.5",

mathesar_ui/src/component-library/common/actions/slider.ts

@@ -128,6 +128,8 @@ export default function slider(
   }
 
   function start(e: MouseEvent | TouchEvent) {
+    e.stopPropagation();
+    e.preventDefault();
     opts.onStart();
     startingValue = opts.getStartingValue();
     startingPosition = getPosition(e, opts.axis);

mathesar_ui/src/component-library/common/utils/ImmutableSet.ts

@@ -32,6 +32,15 @@ export default class ImmutableSet<T> {
     return this.getNewInstance(set);
   }
 
+  /**
+   * @returns a new ImmutableSet that contains only the items that are present
+   * in both this set and the other set. The order of the items in the returned
+   * set is taken from this set (not the supplied set).
+   */
+  intersect(other: { has: (v: T) => boolean }): this {
+    return this.getNewInstance([...this].filter((v) => other.has(v)));
+  }
+
   without(itemOrItems: T | T[]): this {
     const items = Array.isArray(itemOrItems) ? itemOrItems : [itemOrItems];
     const set = new Set(this.set);

mathesar_ui/src/component-library/common/utils/__tests__/ImmutableSet.test.ts

@@ -20,3 +20,15 @@ test('union', () => {
   expect(a.union(empty).valuesArray()).toEqual([2, 7, 13, 19, 5]);
   expect(empty.union(a).valuesArray()).toEqual([2, 7, 13, 19, 5]);
 });
+
+test('intersect', () => {
+  const a = new ImmutableSet([2, 7, 13, 19, 5]);
+  const b = new ImmutableSet([23, 13, 3, 2]);
+  const empty = new ImmutableSet<number>();
+  expect(a.intersect(a).valuesArray()).toEqual([2, 7, 13, 19, 5]);
+  expect(a.intersect(b).valuesArray()).toEqual([2, 13]);
+  expect(b.intersect(a).valuesArray()).toEqual([13, 2]);
+  expect(a.intersect(empty).valuesArray()).toEqual([]);
+  expect(empty.intersect(a).valuesArray()).toEqual([]);
+  expect(empty.intersect(empty).valuesArray()).toEqual([]);
+});

mathesar_ui/src/components/cell-fabric/CellFabric.svelte

@@ -4,25 +4,16 @@
   This component is meant to be common for tables, queries, and for import preview
 -->
 <script lang="ts">
-  import { createEventDispatcher } from 'svelte';
-
   import type { HorizontalAlignment } from './data-types/components/typeDefinitions';
   import type { CellColumnFabric } from './types';
 
-  type CustomEvents = {
-    onSelectionStart: { e: MouseEvent };
-    onMouseEnterCellWhileSelection: { e: MouseEvent };
-  };
-  const dispatch = createEventDispatcher<CustomEvents>();
-
   export let columnFabric: CellColumnFabric;
   export let value: unknown;
   export let recordSummary: string | undefined = undefined;
   export let setRecordSummary:
     | ((recordId: string, recordSummary: string) => void)
     | undefined = undefined;
   export let isActive = false;
-  export let isSelectedInRange = false;
   export let disabled = false;
   export let showAsSkeleton = false;
   export let horizontalAlignment: HorizontalAlignment | undefined = undefined;
@@ -31,34 +22,25 @@
   export let isIndependentOfSheet = false;
   export let showTruncationPopover = false;
   export let canViewLinkedEntities = true;
+  export let lightText = false;
 
   $: ({ cellComponentAndProps } = columnFabric);
   $: ({ component } = cellComponentAndProps);
   $: props = cellComponentAndProps.props as Record<string, unknown>;
-
-  function handleMouseDown(e: MouseEvent) {
-    dispatch('onSelectionStart', { e });
-  }
-
-  function handleMouseEnter(e: MouseEvent) {
-    dispatch('onMouseEnterCellWhileSelection', { e });
-  }
 </script>
 
 <div
   class="cell-fabric"
   data-column-identifier={columnFabric.id}
   class:show-as-skeleton={showAsSkeleton}
   class:is-independent={isIndependentOfSheet}
-  on:mousedown={handleMouseDown}
-  on:mouseenter={handleMouseEnter}
+  class:light-text={lightText}
 >
   <svelte:component
     this={component}
     {...props}
     {columnFabric}
     {isActive}
-    {isSelectedInRange}
     {disabled}
     {isIndependentOfSheet}
     {horizontalAlignment}
@@ -70,9 +52,7 @@
     {canViewLinkedEntities}
     bind:value
     on:movementKeyDown
-    on:activate
     on:update
-    on:mouseenter
   />
 
   <div class="loader">
@@ -118,4 +98,7 @@
   .cell-fabric:not(.show-as-skeleton) .loader {
     display: none;
   }
+  .light-text {
+    color: var(--cell-text-color-processing);
+  }
 </style>

mathesar_ui/src/components/cell-fabric/data-types/components/CellWrapper.svelte

@@ -1,14 +1,15 @@
 <script lang="ts">
-  import { tick } from 'svelte';
+  import { createEventDispatcher } from 'svelte';
 
   import CellBackground from '@mathesar/components/CellBackground.svelte';
   import type { ValueComparisonOutcome } from '@mathesar-component-library/types';
 
   import type { HorizontalAlignment } from './typeDefinitions';
 
+  const dispatch = createEventDispatcher();
+
   export let element: HTMLElement | undefined = undefined;
   export let isActive = false;
-  export let isSelectedInRange = false;
   export let disabled = false;
   export let mode: 'edit' | 'default' = 'default';
   export let multiLineTruncate = false;
@@ -26,58 +27,28 @@
    */
   export let horizontalAlignment: HorizontalAlignment = 'left';
 
-  let isFocused = false;
-
-  function shouldAutoFocus(
-    _isActive: boolean,
-    _mode: 'edit' | 'default',
-  ): boolean {
-    if (!_isActive) {
-      // Don't auto-focus inactive cells
-      return false;
-    }
-    if (_mode === 'edit') {
-      // Don't auto-focus cells in edit mode
-      return false;
-    }
+  /**
+   * This function exists to ensure that the cell is focused after the user
+   * moves from edit mode to default mode via pressing Enter.
+   */
+  function autoFocus() {
     if (!element) {
       // Can't focus if we haven't mounted an element yet
-      return false;
-    }
-    if (element.contains(document.activeElement)) {
-      // Don't auto-focus if the cell contains another element that is already
-      // focused (e.g. an input).
-      return false;
+      return;
     }
-    return true;
-  }
-
-  async function handleStateChange(
-    _isActive: boolean,
-    _mode: 'edit' | 'default',
-  ) {
-    await tick();
-    if (shouldAutoFocus(_isActive, _mode)) {
-      element?.focus();
+    if (!element.contains(document.activeElement)) {
+      // Only auto-focus when the cell contains another element that is already
+      // focused (e.g. an input). If the user moves from edit mode to default
+      // mode via clicking on some UI element outside sheet, then we _don't_
+      // want to focus the cell. We want to keep the focus on the other UI
+      // element that they clicked.
+      return;
     }
-  }
-  $: void handleStateChange(isActive, mode);
-
-  function handleFocus() {
-    isFocused = true;
-    // Note: you might think we ought to automatically activate the cell at this
-    // point to ensure that we don't have any cells which are focused but not
-    // active. I tried this and it caused bugs with selecting columns and rows
-    // via header cells. I didn't want to spend time tracking them down because
-    // we are planning to refactor the cell selection logic soon anyway. It
-    // doesn't _seem_ like we have any code which focuses the cell without
-    // activating it, but it would be nice to eventually build a better
-    // guarantee into the codebase which prevents cells from being focused
-    // without being activated.
+    element.focus();
   }
 
-  function handleBlur() {
-    isFocused = false;
+  $: if (mode === 'default') {
+    autoFocus();
   }
 
   function handleCopy(e: ClipboardEvent) {
@@ -91,12 +62,21 @@
       e.stopPropagation();
     }
   }
+
+  function handleMouseDown(e: MouseEvent) {
+    if (mode === 'edit') {
+      // In edit mode we want to capture mousedown events and prevent them from
+      // propagating to the sheet where mousedown events are used to select
+      // cells. Without this call, clicking inside a cell input would cause the
+      // cell to exit edit mode.
+      e.stopPropagation();
+    }
+    dispatch('mousedown', e);
+  }
 </script>
 
 <div
   class="cell-wrapper"
-  class:is-active={isActive}
-  class:is-focused={isFocused}
   class:disabled
   class:is-edit-mode={mode === 'edit'}
   class:truncate={multiLineTruncate && !isIndependentOfSheet}
@@ -105,20 +85,20 @@
   class:exact-match={valueComparisonOutcome === 'exactMatch'}
   class:substring-match={valueComparisonOutcome === 'substringMatch'}
   class:no-match={valueComparisonOutcome === 'noMatch'}
+  data-active-cell={isActive ? '' : undefined}
   bind:this={element}
   on:click
   on:dblclick
-  on:mousedown
+  on:mousedown={handleMouseDown}
+  on:mouseup
   on:mouseenter
+  on:mouseleave
   on:keydown
   on:copy={handleCopy}
-  on:focus={handleFocus}
-  on:blur={handleBlur}
   tabindex={-1}
   {...$$restProps}
 >
   {#if mode !== 'edit'}
-    <CellBackground color="rgba(14, 101, 235, 0.1)" when={isSelectedInRange} />
     <CellBackground
       color="var(--cell-background-color)"
       when={valueComparisonOutcome !== 'noMatch'}
@@ -144,11 +124,11 @@
       text-align: right;
     }
 
-    &.is-active {
+    &[data-active-cell] {
       box-shadow: 0 0 0 2px var(--slate-300);
       border-radius: 2px;
 
-      &.is-focused {
+      &:focus {
         box-shadow: 0 0 0 2px var(--sky-700);
       }
     }

mathesar_ui/src/components/cell-fabric/data-types/components/SteppedInputCell.svelte

@@ -22,7 +22,6 @@
   type Props = CellTypeProps<Value>;
 
   export let isActive: Props['isActive'];
-  export let isSelectedInRange: Props['isSelectedInRange'];
   export let value: Props['value'];
   export let disabled: Props['disabled'];
   export let multiLineTruncate = false;
@@ -144,23 +143,15 @@
     resetEditMode();
   }
 
-  function handleMouseDown() {
-    if (!isActive) {
-      dispatch('activate');
-    }
-  }
-
   onMount(initLastSavedValue);
 </script>
 
 <CellWrapper
   {isActive}
-  {isSelectedInRange}
   {disabled}
   bind:element={cellRef}
   on:dblclick={setModeToEdit}
   on:keydown={handleKeyDown}
-  on:mousedown={handleMouseDown}
   on:mouseenter
   mode={isEditMode ? 'edit' : 'default'}
   {multiLineTruncate}

mathesar_ui/src/components/cell-fabric/data-types/components/__tests__/SteppedInputCell.test.ts

@@ -5,7 +5,7 @@ import SteppedInputCell from '../SteppedInputCell.svelte';
 
 const requiredProps = {
   isActive: false,
-  isSelectedInRange: false,
+  isSelected: false,
   disabled: false,
 };
 

mathesar_ui/src/components/cell-fabric/data-types/components/array/ArrayCell.svelte

@@ -14,7 +14,6 @@
   const dispatch = createEventDispatcher();
 
   export let isActive: $$Props['isActive'];
-  export let isSelectedInRange: $$Props['isSelectedInRange'];
   export let value: $$Props['value'] = undefined;
   export let disabled: $$Props['disabled'];
   export let isIndependentOfSheet: $$Props['isIndependentOfSheet'];
@@ -36,22 +35,14 @@
         break;
     }
   }
-
-  function handleMouseDown() {
-    if (!isActive) {
-      dispatch('activate');
-    }
-  }
 </script>
 
 <CellWrapper
   {isActive}
-  {isSelectedInRange}
   {disabled}
   {isIndependentOfSheet}
   on:mouseenter
   on:keydown={handleWrapperKeyDown}
-  on:mousedown={handleMouseDown}
 >
   <CellValue {value}>
     {#if isDefinedNonNullable(value)}