fix(core): at menu ux (#10485)

fix AF-2285

1. loading icon will be rendered to the group name
2. make the focused item more stable

blocksuite/affine/block-root/src/widgets/linked-doc/config.ts

@@ -55,12 +55,13 @@ export interface LinkedWidgetConfig {
    *
    * If the return value is not null, no action will be taken.
    */
-  autoFocusedItem?: (
+  autoFocusedItemKey?: (
     menus: LinkedMenuGroup[],
     query: string,
+    currentActiveKey: string | null,
     editorHost: EditorHost,
     inlineEditor: AffineInlineEditor
-  ) => LinkedMenuItem | null;
+  ) => string | null;
 
   mobile: {
     useScreenHeight?: boolean;
@@ -101,8 +102,6 @@ export type LinkedMenuGroup = {
   loading?: boolean | Signal<boolean>;
   // copywriting when display quantity exceeds
   overflowText?: string | Signal<string>;
-  // loading text
-  loadingText?: string | Signal<string>;
 };
 
 export type LinkedDocContext = {

blocksuite/affine/block-root/src/widgets/linked-doc/linked-doc-popover.ts

@@ -75,23 +75,33 @@ export class LinkedDocPopover extends SignalWatcher(
     // need to rebind the effect because this._linkedDocGroup has changed.
     this._menusItemsEffectCleanup = effect(() => {
       this._updateAutoFocusedItem();
+
+      // wait for the next tick to ensure the items are rendered to DOM
+      setTimeout(() => {
+        this.scrollToFocusedItem();
+      });
     });
   };
 
   private readonly _updateAutoFocusedItem = () => {
-    if (!this._query) {
+    // Get the auto-focused item key from the config
-      return;
+    const autoFocusedItemKey = this.context.config.autoFocusedItemKey?.(
-    }
-    const autoFocusedItem = this.context.config.autoFocusedItem?.(
       this._linkedDocGroup,
-      this._query,
+      this._query || '',
+      this._activatedItemKey,
       this.context.std.host,
       this.context.inlineEditor
     );
-    if (autoFocusedItem) {
+
-      this._activatedItemIndex = this._flattenActionList.findIndex(
+    if (autoFocusedItemKey) {
-        item => item.key === autoFocusedItem.key
+      this._activatedItemKey = autoFocusedItemKey;
-      );
+      return;
+    }
+
+    // If no auto-focused item key is returned from the config and no item is currently focused,
+    // focus the first item in the flattened action list
+    if (!this._activatedItemKey && this._flattenActionList.length > 0) {
+      this._activatedItemKey = this._flattenActionList[0].key;
     }
   };
 
@@ -126,19 +136,9 @@ export class LinkedDocPopover extends SignalWatcher(
     let items = resolveSignal(group.items);
 
     const isOverflow = !!group.maxDisplay && items.length > group.maxDisplay;
-    const isLoading = resolveSignal(group.loading);
 
     items = isExpanded ? items : items.slice(0, group.maxDisplay);
 
-    if (isLoading) {
-      items = items.concat({
-        key: 'loading',
-        name: resolveSignal(group.loadingText) || 'loading',
-        icon: LoadingIcon,
-        action: () => {},
-      });
-    }
-
     if (isOverflow && !isExpanded && group.maxDisplay) {
       items = items.concat({
         key: `${group.name} More`,
@@ -183,7 +183,6 @@ export class LinkedDocPopover extends SignalWatcher(
       target: eventSource,
       signal: keydownObserverAbortController.signal,
       onInput: isComposition => {
-        this._activatedItemIndex = 0;
         if (isComposition) {
           this._updateLinkedDocGroup().catch(console.error);
         } else {
@@ -193,7 +192,6 @@ export class LinkedDocPopover extends SignalWatcher(
         }
       },
       onPaste: () => {
-        this._activatedItemIndex = 0;
         setTimeout(() => {
           this._updateLinkedDocGroup().catch(console.error);
         }, 50);
@@ -206,33 +204,18 @@ export class LinkedDocPopover extends SignalWatcher(
         if (curRange.index < this.context.startRange.index) {
           this.context.close();
         }
-        this._activatedItemIndex = 0;
         this.context.inlineEditor.slots.renderComplete.once(
           this._updateLinkedDocGroup
         );
       },
       onMove: step => {
         const itemLen = this._flattenActionList.length;
-        this._activatedItemIndex =
+        const nextIndex = (itemLen + this._activatedItemIndex + step) % itemLen;
-          (itemLen + this._activatedItemIndex + step) % itemLen;
+        const item = this._flattenActionList[nextIndex];
-
+        if (item) {
-        // Scroll to the active item
+          this._activatedItemKey = item.key;
-        const item = this._flattenActionList[this._activatedItemIndex];
-        const shadowRoot = this.shadowRoot;
-        if (!shadowRoot) {
-          console.warn('Failed to find the shadow root!', this);
-          return;
         }
-        const ele = shadowRoot.querySelector(
+        this.scrollToFocusedItem();
-          `icon-button[data-id="${item.key}"]`
-        );
-        if (!ele) {
-          console.warn('Failed to find the active item!', item);
-          return;
-        }
-        ele.scrollIntoView({
-          block: 'nearest',
-        });
       },
       onConfirm: () => {
         this._flattenActionList[this._activatedItemIndex]
@@ -261,19 +244,29 @@ export class LinkedDocPopover extends SignalWatcher(
           visibility: 'hidden',
         });
 
-    // XXX This is a side effect
+    const actionGroups = this._actionGroup.map(group => {
-    let accIdx = 0;
+      // Check if the group is loading
+      const isLoading = resolveSignal(group.loading);
+      return {
+        ...group,
+        isLoading,
+      };
+    });
+
     return html`<div class="linked-doc-popover" style="${style}">
-      ${this._actionGroup
+      ${actionGroups
-        .filter(group => group.items.length)
+        .filter(group => group.items.length || group.isLoading)
         .map((group, idx) => {
           return html`
             <div class="divider" ?hidden=${idx === 0}></div>
-            <div class="group-title">${group.name}</div>
+            <div class="group-title">
+              ${group.name}
+              ${group.isLoading
+                ? html`<span class="loading-icon">${LoadingIcon}</span>`
+                : nothing}
+            </div>
             <div class="group" style=${group.styles ?? ''}>
               ${group.items.map(({ key, name, icon, action }) => {
-                accIdx++;
-                const curIdx = accIdx - 1;
                 const tooltip = this._showTooltip
                   ? html`<affine-tooltip
                       tip-position=${'right'}
@@ -290,13 +283,13 @@ export class LinkedDocPopover extends SignalWatcher(
                   height="30px"
                   data-id=${key}
                   .text=${name}
-                  hover=${this._activatedItemIndex === curIdx}
+                  hover=${this._activatedItemKey === key}
                   @click=${() => {
                     action()?.catch(console.error);
                   }}
                   @mousemove=${() => {
                     // Use `mousemove` instead of `mouseover` to avoid navigate conflict with keyboard
-                    this._activatedItemIndex = curIdx;
+                    this._activatedItemKey = key;
                     // show tooltip whether text length overflows
                     for (const button of this.iconButtons.values()) {
                       if (button.dataset.id == key && button.textElement) {
@@ -348,8 +341,40 @@ export class LinkedDocPopover extends SignalWatcher(
     }
   }
 
+  private scrollToFocusedItem() {
+    const shadowRoot = this.shadowRoot;
+    if (!shadowRoot) {
+      return;
+    }
+
+    // If there's no active item key, don't try to scroll
+    if (!this._activatedItemKey) {
+      return;
+    }
+
+    const ele = shadowRoot.querySelector(
+      `icon-button[data-id="${this._activatedItemKey}"]`
+    );
+
+    // If the element doesn't exist, don't log a warning
+    if (!ele) {
+      return;
+    }
+
+    ele.scrollIntoView({
+      block: 'nearest',
+    });
+  }
+
+  get _activatedItemIndex() {
+    const index = this._flattenActionList.findIndex(
+      item => item.key === this._activatedItemKey
+    );
+    return index === -1 ? 0 : index;
+  }
+
   @state()
-  private accessor _activatedItemIndex = 0;
+  private accessor _activatedItemKey: string | null = null;
 
   @state()
   private accessor _linkedDocGroup: LinkedMenuGroup[] = [];

blocksuite/affine/block-root/src/widgets/linked-doc/styles.ts

@@ -51,6 +51,18 @@ export const linkedDocPopoverStyles = css`
     align-items: center;
     flex-shrink: 0;
     font-weight: 500;
+    justify-content: space-between;
+  }
+
+  .linked-doc-popover .group-title .loading-icon {
+    display: flex;
+    align-items: center;
+    margin-left: 8px;
+  }
+
+  .linked-doc-popover .group-title .loading-icon svg {
+    width: 20px;
+    height: 20px;
   }
 
   .linked-doc-popover .divider {

packages/frontend/core/src/modules/at-menu-config/services/index.ts

@@ -32,6 +32,12 @@ function resolveSignal<T>(data: T | Signal<T>): T {
   return data instanceof Signal ? data.value : data;
 }
 
+const RESERVED_ITEM_KEYS = {
+  createPage: 'create:page',
+  createEdgeless: 'create:edgeless',
+  datePicker: 'date-picker',
+};
+
 export class AtMenuConfigService extends Service {
   constructor(
     private readonly journalService: JournalService,
@@ -50,7 +56,7 @@ export class AtMenuConfigService extends Service {
     return {
       getMenus: this.getMenusFn(),
       mobile: this.getMobileConfig(),
-      autoFocusedItem: this.autoFocusedItem,
+      autoFocusedItemKey: this.autoFocusedItemKey,
     };
   }
 
@@ -61,18 +67,27 @@ export class AtMenuConfigService extends Service {
     });
   }
 
-  private readonly autoFocusedItem = (
+  private readonly autoFocusedItemKey = (
     menus: LinkedMenuGroup[],
-    query: string
+    query: string,
-  ): LinkedMenuItem | null => {
+    currentActiveKey: string | null
+  ): string | null => {
     if (query.trim().length === 0) {
       return null;
     }
+
+    if (
+      currentActiveKey === RESERVED_ITEM_KEYS.createPage ||
+      currentActiveKey === RESERVED_ITEM_KEYS.createEdgeless
+    ) {
+      return currentActiveKey;
+    }
+
     // if the second group (linkToDocGroup) is EMPTY,
     // if the query is NOT empty && the second group (linkToDocGroup) is EMPTY,
     // we will focus on the first item of the third group (create), which is the "New Doc" item.
     if (resolveSignal(menus[1].items).length === 0) {
-      return resolveSignal(menus[2].items)[0];
+      return resolveSignal(menus[2].items)[0]?.key;
     }
     return null;
   };
@@ -95,11 +110,9 @@ export class AtMenuConfigService extends Service {
       ? originalNewDocMenuGroup.items
       : originalNewDocMenuGroup.items.value;
 
-    const newDocItem = items.find(item => item.key === 'create');
     const importItem = items.find(item => item.key === 'import');
 
-    // should have both new doc and import item
+    if (!importItem) {
-    if (!newDocItem || !importItem) {
       return originalNewDocMenuGroup;
     }
 
@@ -117,7 +130,7 @@ export class AtMenuConfigService extends Service {
 
     const customNewDocItems: LinkedMenuItem[] = [
       {
-        key: 'create-page',
+        key: RESERVED_ITEM_KEYS.createPage,
         icon: NewXxxPageIcon(),
         name: I18n.t('com.affine.editor.at-menu.create-page', {
           name: query || I18n.t('Untitled'),
@@ -132,7 +145,7 @@ export class AtMenuConfigService extends Service {
         },
       },
       {
-        key: 'create-edgeless',
+        key: RESERVED_ITEM_KEYS.createEdgeless,
         icon: NewXxxEdgelessIcon(),
         name: I18n.t('com.affine.editor.at-menu.create-edgeless', {
           name: query || I18n.t('Untitled'),
@@ -190,7 +203,7 @@ export class AtMenuConfigService extends Service {
     const items: LinkedMenuItem[] = [
       {
         icon: DateTimeIcon(),
-        key: 'date-picker',
+        key: RESERVED_ITEM_KEYS.datePicker,
         name: I18n.t('com.affine.editor.at-menu.date-picker'),
         action: () => {
           close();
@@ -245,7 +258,7 @@ export class AtMenuConfigService extends Service {
 
       items.unshift({
         icon: icon(),
-        key: dateString,
+        key: RESERVED_ITEM_KEYS.datePicker + ':' + dateString,
         name: alias
           ? html`${alias},
               <span style="color: ${cssVarV2('text/secondary')}"

packages/frontend/core/src/modules/doc-search-menu/services/index.ts

@@ -11,7 +11,7 @@ import { Service } from '@toeverything/infra';
 import { cssVarV2 } from '@toeverything/theme/v2';
 import { html } from 'lit';
 import { unsafeHTML } from 'lit/directives/unsafe-html.js';
-import { map } from 'rxjs';
+import { map, takeWhile } from 'rxjs';
 
 import type { DocDisplayMetaService } from '../../doc-display-meta';
 import type { DocsSearchService } from '../../docs-search';
@@ -105,7 +105,8 @@ export class DocSearchMenuService extends Service {
     const { signal: isIndexerLoading, cleanup: cleanupIndexerLoading } =
       createSignalFromObservable(
         this.docsSearch.indexer.status$.pipe(
-          map(status => status.remaining !== undefined && status.remaining > 0)
+          map(status => status.remaining !== undefined && status.remaining > 0),
+          takeWhile(isLoading => isLoading, true)
         ),
         false
       );
@@ -127,7 +128,6 @@ export class DocSearchMenuService extends Service {
         query,
       }),
       loading: isIndexerLoading,
-      loadingText: I18n.t('com.affine.editor.at-menu.loading'),
       items: docsSignal,
       maxDisplay: MAX_DOCS,
       overflowText,

tests/affine-local/e2e/links.spec.ts

@@ -483,7 +483,7 @@ test('@ popover can auto focus on the "New Doc" item when query returns no items
   await expect(page.locator('.linked-doc-popover')).toBeVisible();
   const newDocMenuItem = page
     .locator('.linked-doc-popover')
-    .locator('[data-id="create-page"]');
+    .locator('[data-id="create:page"]');
   await expect(newDocMenuItem).toBeVisible();
   await expect(newDocMenuItem).toHaveAttribute('hover', 'true');
 });