From ef6717e59a95793ed90fcf9e055a759b7859d582 Mon Sep 17 00:00:00 2001 From: DarkSky <25152247+darkskygit@users.noreply.github.com> Date: Mon, 23 Feb 2026 06:37:16 +0800 Subject: [PATCH] fix(editor): editor behavior and styles (#14498) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix #14269 fix #13920 fix #13977 fix #13953 fix #13895 fix #13905 fix #14136 fix #14357 fix #14491 #### PR Dependency Tree * **PR #14498** ๐Ÿ‘ˆ This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) ## Summary by CodeRabbit * **Bug Fixes** * Callout and toolbar defaults now reliably show grey backgrounds * Keyboard shortcuts behave better across layouts and non-ASCII input * Deleted workspaces no longer appear in local listings * **New Features** * Cell editing now respects pre-entry validation hooks * Scrollbars use themeable variables and include Chromium compatibility fixes * **Style** * Minor UI color adjustment for hidden properties * **Tests** * Added unit tests for table column handling and keymap behavior --- .../blocks/callout/src/callout-block.ts | 6 +- .../blocks/callout/src/configs/toolbar.ts | 4 +- blocksuite/affine/blocks/code/src/styles.ts | 10 ++ .../src/__tests__/table.unit.spec.ts | 52 ++++++++ .../view-presets/table/pc-virtual/row/cell.ts | 4 +- .../src/view-presets/table/pc/cell.ts | 4 +- .../view-presets/table/table-view-manager.ts | 77 +++++++++++- .../shared/src/styles/scrollbar-style.ts | 4 +- .../std/src/__tests__/keymap.unit.spec.ts | 119 ++++++++++++++++++ blocksuite/framework/std/src/event/keymap.ts | 16 ++- .../electron/src/helper/workspace/handlers.ts | 8 ++ .../electron/test/workspace/handlers.spec.ts | 16 +++ .../explorer/display-menu/properties.css.ts | 2 +- 13 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts diff --git a/blocksuite/affine/blocks/callout/src/callout-block.ts b/blocksuite/affine/blocks/callout/src/callout-block.ts index 9252c32dae..b25090bf5e 100644 --- a/blocksuite/affine/blocks/callout/src/callout-block.ts +++ b/blocksuite/affine/blocks/callout/src/callout-block.ts @@ -216,9 +216,13 @@ export class CalloutBlockComponent extends CaptionedBlockComponent - )[backgroundColorName ?? '']; + )[normalizedBackgroundName ?? 'grey']; const iconContent = getIcon(icon); diff --git a/blocksuite/affine/blocks/callout/src/configs/toolbar.ts b/blocksuite/affine/blocks/callout/src/configs/toolbar.ts index 59c7236b91..d021da8f66 100644 --- a/blocksuite/affine/blocks/callout/src/configs/toolbar.ts +++ b/blocksuite/affine/blocks/callout/src/configs/toolbar.ts @@ -68,14 +68,14 @@ const backgroundColorAction = { ${repeat(colors, color => { const isDefault = color === 'default'; const value = isDefault - ? null + ? cssVarV2.block.callout.background.grey : `var(--affine-text-highlight-${color})`; const displayName = `${color} Background`; return html` updateBackground(color)} + @click=${() => updateBackground(isDefault ? 'grey' : color)} > { }); }); +describe('table column materialization', () => { + test('appends missing properties while preserving existing order and state', () => { + const columns = [ + { id: 'status', width: 240, hide: true }, + { id: 'title', width: 320 }, + ]; + + const next = materializeTableColumns(columns, ['title', 'status', 'date']); + + expect(next).toEqual([ + { id: 'status', width: 240, hide: true }, + { id: 'title', width: 320 }, + { id: 'date', width: DEFAULT_COLUMN_WIDTH }, + ]); + }); + + test('drops stale columns that no longer exist in data source', () => { + const columns = [ + { id: 'title', width: 320 }, + { id: 'removed', width: 200, hide: true }, + ]; + + const next = materializeTableColumns(columns, ['title']); + + expect(next).toEqual([{ id: 'title', width: 320 }]); + }); + + test('returns original reference when columns are already materialized', () => { + const columns = [ + { id: 'title', width: 320 }, + { id: 'status', width: 240, hide: true }, + ]; + + const next = materializeTableColumns(columns, ['title', 'status']); + + expect(next).toBe(columns); + }); + + test('supports type-aware default width when materializing missing columns', () => { + const next = materializeTableColumns([], ['title', 'status'], id => + id === 'title' ? 260 : DEFAULT_COLUMN_WIDTH + ); + + expect(next).toEqual([ + { id: 'title', width: 260 }, + { id: 'status', width: DEFAULT_COLUMN_WIDTH }, + ]); + }); +}); + describe('number formatter', () => { test('number format menu should expose all schema formats', () => { const menuFormats = numberFormats.map(format => format.type); diff --git a/blocksuite/affine/data-view/src/view-presets/table/pc-virtual/row/cell.ts b/blocksuite/affine/data-view/src/view-presets/table/pc-virtual/row/cell.ts index d183437fb4..b95dd17e03 100644 --- a/blocksuite/affine/data-view/src/view-presets/table/pc-virtual/row/cell.ts +++ b/blocksuite/affine/data-view/src/view-presets/table/pc-virtual/row/cell.ts @@ -54,7 +54,9 @@ export class DatabaseCellContainer extends SignalWatcher( const selectionView = this.selectionView; if (selectionView) { const selection = selectionView.selection; - if (selection && this.isSelected(selection) && editing) { + const shouldEnterEditMode = + editing && this.cell?.beforeEnterEditMode() !== false; + if (selection && this.isSelected(selection) && shouldEnterEditMode) { selectionView.selection = TableViewAreaSelection.create({ groupKey: this.groupKey, focus: { diff --git a/blocksuite/affine/data-view/src/view-presets/table/pc/cell.ts b/blocksuite/affine/data-view/src/view-presets/table/pc/cell.ts index c4150abf1e..32a0825e68 100644 --- a/blocksuite/affine/data-view/src/view-presets/table/pc/cell.ts +++ b/blocksuite/affine/data-view/src/view-presets/table/pc/cell.ts @@ -57,7 +57,9 @@ export class TableViewCellContainer extends SignalWatcher( const selectionView = this.selectionController; if (selectionView) { const selection = selectionView.selection; - if (selection && this.isSelected(selection) && editing) { + const shouldEnterEditMode = + editing && this.cell?.beforeEnterEditMode() !== false; + if (selection && this.isSelected(selection) && shouldEnterEditMode) { selectionView.selection = TableViewAreaSelection.create({ groupKey: this.groupKey, focus: { diff --git a/blocksuite/affine/data-view/src/view-presets/table/table-view-manager.ts b/blocksuite/affine/data-view/src/view-presets/table/table-view-manager.ts index b4413402d5..fa7cc35d8a 100644 --- a/blocksuite/affine/data-view/src/view-presets/table/table-view-manager.ts +++ b/blocksuite/affine/data-view/src/view-presets/table/table-view-manager.ts @@ -26,6 +26,52 @@ import type { ViewManager } from '../../core/view-manager/view-manager.js'; import { DEFAULT_COLUMN_MIN_WIDTH, DEFAULT_COLUMN_WIDTH } from './consts.js'; import type { TableViewData } from './define.js'; +export const materializeColumnsByPropertyIds = ( + columns: TableColumnData[], + propertyIds: string[], + getDefaultWidth: (id: string) => number = () => DEFAULT_COLUMN_WIDTH +) => { + const needShow = new Set(propertyIds); + const orderedColumns: TableColumnData[] = []; + + for (const column of columns) { + if (needShow.has(column.id)) { + orderedColumns.push(column); + needShow.delete(column.id); + } + } + + for (const id of needShow) { + orderedColumns.push({ id, width: getDefaultWidth(id), hide: undefined }); + } + + return orderedColumns; +}; + +export const materializeTableColumns = ( + columns: TableColumnData[], + propertyIds: string[], + getDefaultWidth?: (id: string) => number +) => { + const nextColumns = materializeColumnsByPropertyIds( + columns, + propertyIds, + getDefaultWidth + ); + const unchanged = + columns.length === nextColumns.length && + columns.every((column, index) => { + const nextColumn = nextColumns[index]; + return ( + nextColumn != null && + column.id === nextColumn.id && + column.hide === nextColumn.hide + ); + }); + + return unchanged ? columns : nextColumns; +}; + export class TableSingleView extends SingleViewBase { propertiesRaw$ = computed(() => { const needShow = new Set(this.dataSource.properties$.value); @@ -220,10 +266,6 @@ export class TableSingleView extends SingleViewBase { return this.data$.value?.mode ?? 'table'; } - constructor(viewManager: ViewManager, viewId: string) { - super(viewManager, viewId); - } - isShow(rowId: string): boolean { if (this.filter$.value?.conditions.length) { const rowMap = Object.fromEntries( @@ -290,6 +332,33 @@ export class TableSingleView extends SingleViewBase { }); } ); + + private materializeColumns() { + const data = this.data$.value; + if (!data) { + return; + } + + const nextColumns = materializeTableColumns( + data.columns, + this.dataSource.properties$.value, + id => this.propertyGetOrCreate(id).width$.value + ); + if (nextColumns === data.columns) { + return; + } + + this.dataUpdate(() => ({ columns: nextColumns })); + } + + constructor(viewManager: ViewManager, viewId: string) { + super(viewManager, viewId); + // Materialize view columns on view activation so newly added properties + // can participate in hide/order operations in table. + queueMicrotask(() => { + this.materializeColumns(); + }); + } } type TableColumnData = TableViewData['columns'][number]; diff --git a/blocksuite/affine/shared/src/styles/scrollbar-style.ts b/blocksuite/affine/shared/src/styles/scrollbar-style.ts index 44b4d78460..0c5f9b76a7 100644 --- a/blocksuite/affine/shared/src/styles/scrollbar-style.ts +++ b/blocksuite/affine/shared/src/styles/scrollbar-style.ts @@ -1,5 +1,7 @@ import { css, unsafeCSS } from 'lit'; +import { unsafeCSSVarV2 } from '../theme/css-variables'; + /** * You should add a container before the scrollbar style to prevent the style pollution of the whole doc. */ @@ -28,7 +30,7 @@ export const scrollbarStyle = (container: string) => { } ${unsafeCSS(container)}::-webkit-scrollbar-thumb { border-radius: 2px; - background-color: #b1b1b1; + background-color: ${unsafeCSSVarV2('icon/secondary', '#b1b1b1')}; } ${unsafeCSS(container)}::-webkit-scrollbar-corner { display: none; diff --git a/blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts b/blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts new file mode 100644 index 0000000000..e9c0ba05b7 --- /dev/null +++ b/blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts @@ -0,0 +1,119 @@ +import { describe, expect, test } from 'vitest'; + +import { bindKeymap } from '../event/keymap.js'; + +const createKeyboardEvent = (options: { + key: string; + keyCode: number; + altKey?: boolean; + ctrlKey?: boolean; + metaKey?: boolean; + shiftKey?: boolean; +}): KeyboardEvent => { + const event = new KeyboardEvent('keydown', { + key: options.key, + altKey: options.altKey ?? false, + ctrlKey: options.ctrlKey ?? false, + metaKey: options.metaKey ?? false, + shiftKey: options.shiftKey ?? false, + }); + + Object.defineProperty(event, 'keyCode', { + configurable: true, + get: () => options.keyCode, + }); + Object.defineProperty(event, 'which', { + configurable: true, + get: () => options.keyCode, + }); + + return event; +}; + +const createCtx = (event: KeyboardEvent) => { + return { + get(name: string) { + if (name === 'keyboardState') { + return { raw: event }; + } + return undefined; + }, + } as any; +}; + +describe('bindKeymap', () => { + test('falls back to physical key for ctrl shortcuts on non-US layouts', () => { + let handled = false; + const handler = bindKeymap({ + 'Ctrl-f': () => { + handled = true; + return true; + }, + }); + + const event = createKeyboardEvent({ + key: 'ะฐ', + keyCode: 70, + ctrlKey: true, + }); + + expect(handler(createCtx(event))).toBe(true); + expect(handled).toBe(true); + }); + + test('does not fallback for Alt+locale-character letter input', () => { + let handled = false; + const handler = bindKeymap({ + 'Alt-s': () => { + handled = true; + return true; + }, + }); + + const event = createKeyboardEvent({ + key: 'ล›', + keyCode: 83, + altKey: true, + }); + + expect(handler(createCtx(event))).toBe(false); + expect(handled).toBe(false); + }); + + test('keeps Alt+digit fallback for non-ASCII key outputs', () => { + let handled = false; + const handler = bindKeymap({ + 'Alt-0': () => { + handled = true; + return true; + }, + }); + + const event = createKeyboardEvent({ + key: 'ยบ', + keyCode: 48, + altKey: true, + }); + + expect(handler(createCtx(event))).toBe(true); + expect(handled).toBe(true); + }); + + test('does not fallback on non-ASCII input without modifiers', () => { + let handled = false; + const handler = bindKeymap({ + '[': () => { + handled = true; + return true; + }, + }); + + const event = createKeyboardEvent({ + key: 'ั…', + keyCode: 219, + }); + + expect(handler(createCtx(event))).toBe(false); + expect(handled).toBe(false); + }); +}); diff --git a/blocksuite/framework/std/src/event/keymap.ts b/blocksuite/framework/std/src/event/keymap.ts index 7887f7b995..e4ef40163d 100644 --- a/blocksuite/framework/std/src/event/keymap.ts +++ b/blocksuite/framework/std/src/event/keymap.ts @@ -90,9 +90,21 @@ export function bindKeymap( // Do NOT fallback when the key produces a non-ASCII character (e.g., Cyrillic 'ั…' on Russian keyboard), // because the user intends to type that character, not trigger a shortcut bound to the physical key. // See: https://github.com/toeverything/AFFiNE/issues/14059 - const hasModifier = event.shiftKey || event.altKey || event.metaKey; + const hasModifier = + event.shiftKey || event.altKey || event.ctrlKey || event.metaKey; const baseName = base[event.keyCode]; - if (hasModifier && baseName && baseName !== name) { + const isSingleAscii = name.length === 1 && name.charCodeAt(0) <= 0x7e; + const isAltInputChar = event.altKey && !event.ctrlKey && !isSingleAscii; + // Keep supporting existing Alt+digit shortcuts (e.g. Alt-0/1/2 in edgeless) + // while preventing Alt-based locale input characters from triggering letter shortcuts. + const isDigitBaseKey = + baseName != null && baseName.length === 1 && /[0-9]/.test(baseName); + if ( + hasModifier && + baseName && + baseName !== name && + !(isAltInputChar && !isDigitBaseKey) + ) { const fromCode = map[modifiers(baseName, event)]; if (fromCode && fromCode(ctx)) { return true; diff --git a/packages/frontend/apps/electron/src/helper/workspace/handlers.ts b/packages/frontend/apps/electron/src/helper/workspace/handlers.ts index 9790f393c1..eee2ec9419 100644 --- a/packages/frontend/apps/electron/src/helper/workspace/handlers.ts +++ b/packages/frontend/apps/electron/src/helper/workspace/handlers.ts @@ -106,9 +106,17 @@ export async function listLocalWorkspaceIds(): Promise { return []; } + const deletedWorkspaceBasePath = await getDeletedWorkspacesBasePath(); + const deletedWorkspaceIds = new Set( + (await fs.readdir(deletedWorkspaceBasePath).catch(() => [])).filter(Boolean) + ); + const entries = await fs.readdir(localWorkspaceBasePath); const ids = await Promise.all( entries.map(async entry => { + if (deletedWorkspaceIds.has(entry)) { + return null; + } const workspacePath = path.join(localWorkspaceBasePath, entry); const stat = await fs.stat(workspacePath).catch(() => null); if (!stat?.isDirectory()) { diff --git a/packages/frontend/apps/electron/test/workspace/handlers.spec.ts b/packages/frontend/apps/electron/test/workspace/handlers.spec.ts index 5c3cd14343..58e644fdc4 100644 --- a/packages/frontend/apps/electron/test/workspace/handlers.spec.ts +++ b/packages/frontend/apps/electron/test/workspace/handlers.spec.ts @@ -38,6 +38,7 @@ describe('workspace db management', () => { await import('@affine/electron/helper/workspace/handlers'); const validWorkspaceId = v4(); const noDbWorkspaceId = v4(); + const deletedWorkspaceId = v4(); const fileEntry = 'README.txt'; const validWorkspacePath = path.join( @@ -52,6 +53,17 @@ describe('workspace db management', () => { 'local', noDbWorkspaceId ); + const deletedWorkspacePath = path.join( + appDataPath, + 'workspaces', + 'local', + deletedWorkspaceId + ); + const deletedWorkspaceTrashPath = path.join( + appDataPath, + 'deleted-workspaces', + deletedWorkspaceId + ); const nonDirectoryPath = path.join( appDataPath, 'workspaces', @@ -62,11 +74,15 @@ describe('workspace db management', () => { await fs.ensureDir(validWorkspacePath); await fs.ensureFile(path.join(validWorkspacePath, 'storage.db')); await fs.ensureDir(noDbWorkspacePath); + await fs.ensureDir(deletedWorkspacePath); + await fs.ensureFile(path.join(deletedWorkspacePath, 'storage.db')); + await fs.ensureDir(deletedWorkspaceTrashPath); await fs.outputFile(nonDirectoryPath, 'not-a-workspace'); const ids = await listLocalWorkspaceIds(); expect(ids).toContain(validWorkspaceId); expect(ids).not.toContain(noDbWorkspaceId); + expect(ids).not.toContain(deletedWorkspaceId); expect(ids).not.toContain(fileEntry); }); diff --git a/packages/frontend/core/src/components/explorer/display-menu/properties.css.ts b/packages/frontend/core/src/components/explorer/display-menu/properties.css.ts index b9209233d8..fd4d554e46 100644 --- a/packages/frontend/core/src/components/explorer/display-menu/properties.css.ts +++ b/packages/frontend/core/src/components/explorer/display-menu/properties.css.ts @@ -25,7 +25,7 @@ export const property = style({ selectors: { '&[data-show="false"]': { backgroundColor: cssVarV2.button.emptyIconBackground, - color: cssVarV2.icon.disable, + color: cssVarV2.text.secondary, }, }, });