refactor(core): use fractional indexing for sorting (#5809)

use https://github.com/rocicorp/fractional-indexing to enable better sorting logic for crdt app

packages/frontend/core/package.json

@@ -61,6 +61,7 @@
     "cssnano": "^6.0.1",
     "dayjs": "^1.11.10",
     "foxact": "^0.2.20",
+    "fractional-indexing": "^3.2.0",
     "graphql": "^16.8.1",
     "idb": "^8.0.0",
     "image-blob-reduce": "^4.1.0",

packages/frontend/core/src/components/affine/page-properties/page-properties-manager.ts

@@ -6,6 +6,7 @@ import type {
 } from '@affine/core/modules/workspace/properties/schema';
 import { PagePropertyType } from '@affine/core/modules/workspace/properties/schema';
 import { DebugLogger } from '@affine/debug';
+import { generateKeyBetween } from 'fractional-indexing';
 import { nanoid } from 'nanoid';
 
 import { getDefaultIconName } from './icons-mapping';
@@ -216,16 +217,13 @@ export class PagePropertiesManager {
   }
 
   getOrderedCustomProperties() {
-    return Object.values(this.getCustomProperties()).sort(
+    return Object.values(this.getCustomProperties()).sort((a, b) =>
-      (a, b) => a.order - b.order
+      a.order > b.order ? 1 : a.order < b.order ? -1 : 0
     );
   }
 
   largestOrder() {
-    return Math.max(
+    return this.getOrderedCustomProperties().at(-1)?.order ?? null;
-      ...Object.values(this.properties.custom).map(p => p.order),
-      0
-    );
   }
 
   getCustomPropertyMeta(id: string): PageInfoCustomPropertyMeta | undefined {
@@ -247,7 +245,7 @@ export class PagePropertiesManager {
       return;
     }
 
-    const newOrder = this.largestOrder() + 1;
+    const newOrder = generateKeyBetween(this.largestOrder(), null);
     if (this.properties.custom[id]) {
       logger.warn(`custom property ${id} already exists`);
     }
@@ -260,6 +258,20 @@ export class PagePropertiesManager {
     };
   }
 
+  moveCustomProperty(from: number, to: number) {
+    // move from -> to means change from's order to a new order between to and to -1/+1
+    const properties = this.getOrderedCustomProperties();
+    const fromProperty = properties[from];
+    const toProperty = properties[to];
+    const toNextProperty = properties[from < to ? to + 1 : to - 1];
+    const args: [string?, string?] =
+      from < to
+        ? [toProperty.order, toNextProperty?.order ?? null]
+        : [toNextProperty?.order ?? null, toProperty.order];
+    const newOrder = generateKeyBetween(...args);
+    this.properties.custom[fromProperty.id].order = newOrder;
+  }
+
   hasCustomProperty(id: string) {
     return !!this.properties.custom[id];
   }

packages/frontend/core/src/components/affine/page-properties/table.tsx

@@ -40,7 +40,7 @@ import {
   restrictToParentElement,
   restrictToVerticalAxis,
 } from '@dnd-kit/modifiers';
-import { arrayMove, SortableContext, useSortable } from '@dnd-kit/sortable';
+import { SortableContext, useSortable } from '@dnd-kit/sortable';
 import * as Collapsible from '@radix-ui/react-collapsible';
 import clsx from 'clsx';
 import { use } from 'foxact/use';
@@ -54,6 +54,7 @@ import {
   Suspense,
   useCallback,
   useContext,
+  useEffect,
   useMemo,
   useState,
 } from 'react';
@@ -95,7 +96,12 @@ type PropertyVisibility = PageInfoCustomProperty['visibility'];
 const editingPropertyAtom = atom<string | null>(null);
 
 const modifiers = [restrictToParentElement, restrictToVerticalAxis];
-const SortableProperties = ({ children }: PropsWithChildren) => {
+
+interface SortablePropertiesProps {
+  children: (properties: PageInfoCustomProperty[]) => React.ReactNode;
+}
+
+const SortableProperties = ({ children }: SortablePropertiesProps) => {
   const manager = useContext(managerContext);
   const properties = useMemo(
     () => manager.getOrderedCustomProperties(),
@@ -110,6 +116,14 @@ const SortableProperties = ({ children }: PropsWithChildren) => {
       },
     })
   );
+  // use localProperties since changes applied to upstream may be delayed
+  // if we use that one, there will be weird behavior after reordering
+  const [localProperties, setLocalProperties] = useState(properties);
+
+  useEffect(() => {
+    setLocalProperties(properties);
+  }, [properties]);
+
   const onDragEnd = useCallback(
     (event: DragEndEvent) => {
       if (!draggable) {
@@ -120,22 +134,22 @@ const SortableProperties = ({ children }: PropsWithChildren) => {
       const toIndex = properties.findIndex(p => p.id === over?.id);
 
       if (fromIndex !== -1 && toIndex !== -1 && fromIndex !== toIndex) {
-        const newOrdered = arrayMove(properties, fromIndex, toIndex);
+        manager.moveCustomProperty(fromIndex, toIndex);
-        manager.transact(() => {
+        setLocalProperties(manager.getOrderedCustomProperties());
-          newOrdered.forEach((p, i) => {
-            manager.updateCustomProperty(p.id, {
-              order: i,
-            });
-          });
-        });
       }
     },
     [manager, properties, draggable]
   );
+
+  const filteredProperties = useMemo(
+    () => localProperties.filter(p => manager.getCustomPropertyMeta(p.id)),
+    [localProperties, manager]
+  );
+
   return (
     <DndContext sensors={sensors} onDragEnd={onDragEnd} modifiers={modifiers}>
       <SortableContext disabled={!draggable} items={properties}>
-        {children}
+        {children(filteredProperties)}
       </SortableContext>
     </DndContext>
   );
@@ -167,6 +181,7 @@ const SortablePropertyRow = ({
     transition,
     active,
     isDragging,
+    isSorting,
   } = useSortable({
     id: property.id,
   });
@@ -175,10 +190,10 @@ const SortablePropertyRow = ({
       transform: transform
         ? `translate3d(${transform.x}px, ${transform.y}px, 0)`
         : undefined,
-      transition,
+      transition: isSorting ? transition : undefined,
       pointerEvents: manager.readonly ? 'none' : undefined,
     }),
-    [manager.readonly, transform, transition]
+    [isSorting, manager.readonly, transform, transition]
   );
 
   return (
@@ -305,7 +320,6 @@ export const PagePropertiesSettingsPopup = ({
 }: PagePropertiesSettingsPopupProps) => {
   const manager = useContext(managerContext);
   const t = useAFFiNEI18N();
-  const properties = manager.getOrderedCustomProperties();
 
   const menuItems = useMemo(() => {
     const options: MenuItemOption[] = [];
@@ -321,35 +335,37 @@ export const PagePropertiesSettingsPopup = ({
     options.push('-');
     options.push([
       <SortableProperties key="sortable-settings">
-        {properties.map(property => {
+        {properties =>
-          const meta = manager.getCustomPropertyMeta(property.id);
+          properties.map(property => {
-          assertExists(meta, 'meta should exist for property');
+            const meta = manager.getCustomPropertyMeta(property.id);
-          const Icon = nameToIcon(meta.icon, meta.type);
+            assertExists(meta, 'meta should exist for property');
-          const name = meta.name;
+            const Icon = nameToIcon(meta.icon, meta.type);
-          return (
+            const name = meta.name;
-            <SortablePropertyRow
+            return (
-              key={meta.id}
+              <SortablePropertyRow
-              property={property}
+                key={meta.id}
-              className={styles.propertySettingRow}
+                property={property}
-              data-testid="page-properties-settings-menu-item"
+                className={styles.propertySettingRow}
-            >
+                data-testid="page-properties-settings-menu-item"
-              <MenuIcon>
-                <Icon />
-              </MenuIcon>
-              <div
-                data-testid="page-property-setting-row-name"
-                className={styles.propertyRowName}
               >
-                {name}
+                <MenuIcon>
-              </div>
+                  <Icon />
-              <VisibilityModeSelector property={property} />
+                </MenuIcon>
-            </SortablePropertyRow>
+                <div
-          );
+                  data-testid="page-property-setting-row-name"
-        })}
+                  className={styles.propertyRowName}
+                >
+                  {name}
+                </div>
+                <VisibilityModeSelector property={property} />
+              </SortablePropertyRow>
+            );
+          })
+        }
       </SortableProperties>,
     ]);
     return renderMenuItemOptions(options);
-  }, [manager, properties, t]);
+  }, [manager, t]);
 
   return (
     <Menu
@@ -423,6 +439,14 @@ export const PagePropertyRowNameMenu = ({
 
   const [showDeleteModal, setShowDeleteModal] = useState(false);
 
+  useEffect(() => {
+    setLocalPropertyMeta(meta);
+  }, [meta]);
+
+  useEffect(() => {
+    setLocalProperty(property);
+  }, [property]);
+
   const handleFinishEditing = useCallback(() => {
     onFinishEditing();
     manager.updateCustomPropertyMeta(meta.id, localPropertyMeta);
@@ -752,10 +776,6 @@ export const PagePropertiesTableBody = ({
   style,
 }: PagePropertiesTableBodyProps) => {
   const manager = useContext(managerContext);
-
-  const properties = useMemo(() => {
-    return manager.getOrderedCustomProperties();
-  }, [manager]);
   return (
     <Collapsible.Content
       className={clsx(styles.tableBodyRoot, className)}
@@ -764,16 +784,20 @@ export const PagePropertiesTableBody = ({
       <PageTagsRow />
       <div className={styles.tableBodySortable}>
         <SortableProperties>
-          {properties
+          {properties =>
-            .filter(
+            properties
-              property =>
+              .filter(
-                manager.isPropertyRequired(property.id) ||
+                property =>
-                (property.visibility !== 'hide' &&
+                  manager.isPropertyRequired(property.id) ||
-                  !(property.visibility === 'hide-if-empty' && !property.value))
+                  (property.visibility !== 'hide' &&
-            )
+                    !(
-            .map(property => (
+                      property.visibility === 'hide-if-empty' && !property.value
-              <PagePropertyRow key={property.id} property={property} />
+                    ))
-            ))}
+              )
+              .map(property => (
+                <PagePropertyRow key={property.id} property={property} />
+              ))
+          }
         </SortableProperties>
       </div>
       {manager.readonly ? null : <PagePropertiesAddProperty />}

packages/frontend/core/src/components/affine/page-properties/tags-inline-editor.css.ts

@@ -27,7 +27,7 @@ export const inlineTagsContainer = style({
 export const tagsMenu = style({
   padding: 0,
   transform:
-    'translate(-3px, calc(-3px + var(--radix-popper-anchor-height) * -1))',
+    'translate(-3.5px, calc(-3.5px + var(--radix-popper-anchor-height) * -1))',
   width: 'calc(var(--radix-popper-anchor-width) + 16px)',
   overflow: 'hidden',
 });

packages/frontend/core/src/hooks/use-affine-adapter.ts

@@ -1,33 +1,36 @@
 import type { Workspace } from '@toeverything/infra';
 import { useService } from '@toeverything/infra/di';
-import { use } from 'foxact/use';
+import { useDebouncedState } from 'foxact/use-debounced-state';
-import { useEffect, useMemo, useState } from 'react';
+import { useEffect, useMemo } from 'react';
 
 import { WorkspacePropertiesAdapter } from '../modules/workspace/properties';
-import { useAllBlockSuitePageMeta } from './use-all-block-suite-page-meta';
 
 function getProxy<T extends object>(obj: T) {
   return new Proxy(obj, {});
 }
 
 const useReactiveAdapter = (adapter: WorkspacePropertiesAdapter) => {
-  use(adapter.workspace.blockSuiteWorkspace.doc.whenSynced);
+  // hack: delay proxy creation to avoid unnecessary re-render + render in another component issue
-  const [proxy, setProxy] = useState(adapter);
+  const [proxy, setProxy] = useDebouncedState(adapter, 0);
-  // fixme: this is a hack to force re-render when default meta changed
-  useAllBlockSuitePageMeta(adapter.workspace.blockSuiteWorkspace);
   useEffect(() => {
     // todo: track which properties are used and then filter by property path change
     // using Y.YEvent.path
     function observe() {
-      requestAnimationFrame(() => {
+      setProxy(getProxy(adapter));
-        setProxy(getProxy(adapter));
-      });
     }
+    const disposables: (() => void)[] = [];
+    disposables.push(
+      adapter.workspace.blockSuiteWorkspace.meta.pageMetasUpdated.on(observe)
+        .dispose
+    );
     adapter.properties.observeDeep(observe);
+    disposables.push(() => adapter.properties.unobserveDeep(observe));
     return () => {
-      adapter.properties.unobserveDeep(observe);
+      for (const dispose of disposables) {
+        dispose();
+      }
     };
-  }, [adapter]);
+  }, [adapter, setProxy]);
 
   return proxy;
 };

packages/frontend/core/src/modules/workspace/properties/schema.ts

@@ -88,7 +88,7 @@ const WorkspaceAffinePropertiesSchemaSchema = z.object({
 });
 
 const PageInfoCustomPropertyItemSchema = PageInfoItemSchema.extend({
-  order: z.number(),
+  order: z.string(),
 });
 
 const WorkspacePagePropertiesSchema = z.object({

tests/affine-local/e2e/page-properties.spec.ts

@@ -241,3 +241,45 @@ test('create a required property', async ({ page }) => {
     )
   ).toContainText('Required');
 });
+
+test('delete a required property', async ({ page }) => {
+  await openWorkspaceProperties(page);
+  await addCustomProperty(page, 'Text', true);
+
+  await page
+    .locator('[data-testid="custom-property-row"]:has-text("Text")')
+    .getByRole('button')
+    .click();
+
+  await page
+    .getByRole('menuitem', {
+      name: 'Set as required property',
+    })
+    .click();
+
+  await page
+    .locator('[data-testid="custom-property-row"]:has-text("Text")')
+    .getByRole('button')
+    .click();
+
+  await page
+    .getByRole('menuitem', {
+      name: 'Delete property',
+    })
+    .click();
+  await page
+    .getByRole('button', {
+      name: 'Confirm',
+    })
+    .click();
+
+  // close workspace settings
+  await page.keyboard.press('Escape');
+
+  await waitForEditorLoad(page);
+
+  // check if the property is removed from page properties
+  await expect(
+    page.locator('[data-testid="page-property-row-name"]:has-text("Text")')
+  ).not.toBeVisible();
+});

yarn.lock

@@ -354,6 +354,7 @@ __metadata:
     express: "npm:^4.18.2"
     fake-indexeddb: "npm:^5.0.0"
     foxact: "npm:^0.2.20"
+    fractional-indexing: "npm:^3.2.0"
     graphql: "npm:^16.8.1"
     html-webpack-plugin: "npm:^5.5.3"
     idb: "npm:^8.0.0"