mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-03-24 16:18:39 +08:00
fix(editor): editor behavior and styles (#14498)
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) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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 <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -216,9 +216,13 @@ export class CalloutBlockComponent extends CaptionedBlockComponent<CalloutBlockM
|
||||
override renderBlock() {
|
||||
const icon = this.model.props.icon$.value;
|
||||
const backgroundColorName = this.model.props.backgroundColorName$.value;
|
||||
const normalizedBackgroundName =
|
||||
backgroundColorName === 'default' || backgroundColorName === ''
|
||||
? 'grey'
|
||||
: backgroundColorName;
|
||||
const backgroundColor = (
|
||||
cssVarV2.block.callout.background as Record<string, string>
|
||||
)[backgroundColorName ?? ''];
|
||||
)[normalizedBackgroundName ?? 'grey'];
|
||||
|
||||
const iconContent = getIcon(icon);
|
||||
|
||||
|
||||
@@ -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`
|
||||
<editor-menu-action
|
||||
data-testid="background-${color}"
|
||||
@click=${() => updateBackground(color)}
|
||||
@click=${() => updateBackground(isDefault ? 'grey' : color)}
|
||||
>
|
||||
<affine-text-duotone-icon
|
||||
style=${styleMap({
|
||||
|
||||
@@ -27,6 +27,16 @@ export const codeBlockStyles = css`
|
||||
|
||||
${scrollbarStyle('.affine-code-block-container rich-text')}
|
||||
|
||||
/* In Chromium 121+, non-auto scrollbar-width/color override ::-webkit-scrollbar styles. */
|
||||
@supports not selector(::-webkit-scrollbar) {
|
||||
.affine-code-block-container rich-text {
|
||||
scrollbar-width: thin;
|
||||
scrollbar-color: ${unsafeCSSVarV2('icon/secondary', '#b1b1b1')}
|
||||
transparent;
|
||||
scrollbar-gutter: stable both-edges;
|
||||
}
|
||||
}
|
||||
|
||||
.affine-code-block-container .inline-editor {
|
||||
font-family: var(--affine-font-code-family);
|
||||
font-variant-ligatures: none;
|
||||
|
||||
@@ -6,10 +6,12 @@ import {
|
||||
NumberFormatSchema,
|
||||
parseNumber,
|
||||
} from '../property-presets/number/utils/formatter.js';
|
||||
import { DEFAULT_COLUMN_WIDTH } from '../view-presets/table/consts.js';
|
||||
import { mobileEffects } from '../view-presets/table/mobile/effect.js';
|
||||
import type { MobileTableGroup } from '../view-presets/table/mobile/group.js';
|
||||
import { pcEffects } from '../view-presets/table/pc/effect.js';
|
||||
import type { TableGroup } from '../view-presets/table/pc/group.js';
|
||||
import { materializeTableColumns } from '../view-presets/table/table-view-manager.js';
|
||||
|
||||
/** @vitest-environment happy-dom */
|
||||
|
||||
@@ -41,6 +43,56 @@ describe('TableGroup', () => {
|
||||
});
|
||||
});
|
||||
|
||||
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);
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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<TableViewData> {
|
||||
propertiesRaw$ = computed(() => {
|
||||
const needShow = new Set(this.dataSource.properties$.value);
|
||||
@@ -220,10 +266,6 @@ export class TableSingleView extends SingleViewBase<TableViewData> {
|
||||
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<TableViewData> {
|
||||
});
|
||||
}
|
||||
);
|
||||
|
||||
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];
|
||||
|
||||
@@ -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;
|
||||
|
||||
119
blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts
Normal file
119
blocksuite/framework/std/src/__tests__/keymap.unit.spec.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -106,9 +106,17 @@ export async function listLocalWorkspaceIds(): Promise<string[]> {
|
||||
return [];
|
||||
}
|
||||
|
||||
const deletedWorkspaceBasePath = await getDeletedWorkspacesBasePath();
|
||||
const deletedWorkspaceIds = new Set<string>(
|
||||
(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()) {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ export const property = style({
|
||||
selectors: {
|
||||
'&[data-show="false"]': {
|
||||
backgroundColor: cssVarV2.button.emptyIconBackground,
|
||||
color: cssVarV2.icon.disable,
|
||||
color: cssVarV2.text.secondary,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user