fix(editor): single-letter tags in select/multi-select table cell (#14808)
### Summary of Changes Resolves #14715 and #14280. When a user types into a **Select/Multi-Select** table cell to create/choose a tag, that character is stashed on the cell container (setTagDraft) instead of going through valueSetFromString. Opening the tag picker reads it via consumeTagDraftFromTableCellHost. ### Verification - Added unit test to check that single-character input doesn't immediately call valueSetFromString. https://github.com/user-attachments/assets/432b2693-52f9-4ab4-a694-8440aea007a3 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Tag selection popups now initialize with draft text from keypresses in tag columns, improving user experience when editing tags. * **Tests** * Added comprehensive hotkey tests for single-select and multi-select tag column behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -1,5 +1,7 @@
|
|||||||
import { describe, expect, it, vi } from 'vitest';
|
import { describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
|
import { multiSelectPropertyType } from '../property-presets/multi-select/define.js';
|
||||||
|
import { selectPropertyType } from '../property-presets/select/define.js';
|
||||||
import { TableHotkeysController } from '../view-presets/table/pc/controller/hotkeys.js';
|
import { TableHotkeysController } from '../view-presets/table/pc/controller/hotkeys.js';
|
||||||
import { TableHotkeysController as VirtualHotkeysController } from '../view-presets/table/pc-virtual/controller/hotkeys.js';
|
import { TableHotkeysController as VirtualHotkeysController } from '../view-presets/table/pc-virtual/controller/hotkeys.js';
|
||||||
import {
|
import {
|
||||||
@@ -7,6 +9,11 @@ import {
|
|||||||
TableViewRowSelection,
|
TableViewRowSelection,
|
||||||
} from '../view-presets/table/selection';
|
} from '../view-presets/table/selection';
|
||||||
|
|
||||||
|
const TAG_COLUMN_TYPES = [
|
||||||
|
selectPropertyType.type,
|
||||||
|
multiSelectPropertyType.type,
|
||||||
|
] as const;
|
||||||
|
|
||||||
function createLogic() {
|
function createLogic() {
|
||||||
const view = {
|
const view = {
|
||||||
rowsDelete: vi.fn(),
|
rowsDelete: vi.fn(),
|
||||||
@@ -66,7 +73,10 @@ describe('TableHotkeysController', () => {
|
|||||||
const cell = {
|
const cell = {
|
||||||
rowId: 'r1',
|
rowId: 'r1',
|
||||||
dataset: { rowId: 'r1', columnId: 'c1' },
|
dataset: { rowId: 'r1', columnId: 'c1' },
|
||||||
column: { valueSetFromString: vi.fn() },
|
column: {
|
||||||
|
valueSetFromString: vi.fn(),
|
||||||
|
type$: { value: 'text' },
|
||||||
|
},
|
||||||
};
|
};
|
||||||
selectionController.getCellContainer.mockReturnValue(cell);
|
selectionController.getCellContainer.mockReturnValue(cell);
|
||||||
selectionController.selection = TableViewAreaSelection.create({
|
selectionController.selection = TableViewAreaSelection.create({
|
||||||
@@ -85,6 +95,41 @@ describe('TableHotkeysController', () => {
|
|||||||
expect(selectionController.selection.isEditing).toBe(true);
|
expect(selectionController.selection.isEditing).toBe(true);
|
||||||
expect(evt.preventDefault).toHaveBeenCalled();
|
expect(evt.preventDefault).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each(TAG_COLUMN_TYPES)(
|
||||||
|
'stages draft for %s column instead of valueSetFromString',
|
||||||
|
columnType => {
|
||||||
|
const { logic, selectionController } = createLogic();
|
||||||
|
const ctrl = new TableHotkeysController(logic as any);
|
||||||
|
ctrl.hostConnected();
|
||||||
|
const setTagDraft = vi.fn();
|
||||||
|
const cell = {
|
||||||
|
rowId: 'r1',
|
||||||
|
dataset: { rowId: 'r1', columnId: 'c1' },
|
||||||
|
column: {
|
||||||
|
valueSetFromString: vi.fn(),
|
||||||
|
type$: { value: columnType },
|
||||||
|
},
|
||||||
|
setTagDraft,
|
||||||
|
};
|
||||||
|
selectionController.getCellContainer.mockReturnValue(cell);
|
||||||
|
selectionController.selection = TableViewAreaSelection.create({
|
||||||
|
focus: { rowIndex: 0, columnIndex: 0 },
|
||||||
|
isEditing: false,
|
||||||
|
});
|
||||||
|
const evt = {
|
||||||
|
key: 'C',
|
||||||
|
metaKey: false,
|
||||||
|
ctrlKey: false,
|
||||||
|
altKey: false,
|
||||||
|
preventDefault: vi.fn(),
|
||||||
|
};
|
||||||
|
logic.keyDown({ get: () => ({ raw: evt }) });
|
||||||
|
expect(cell.column.valueSetFromString).not.toHaveBeenCalled();
|
||||||
|
expect(setTagDraft).toHaveBeenCalledWith('C');
|
||||||
|
expect(selectionController.selection.isEditing).toBe(true);
|
||||||
|
}
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Virtual TableHotkeysController', () => {
|
describe('Virtual TableHotkeysController', () => {
|
||||||
@@ -95,7 +140,12 @@ describe('Virtual TableHotkeysController', () => {
|
|||||||
const cell = {
|
const cell = {
|
||||||
rowId: 'r1',
|
rowId: 'r1',
|
||||||
dataset: { rowId: 'r1', columnId: 'c1' },
|
dataset: { rowId: 'r1', columnId: 'c1' },
|
||||||
column$: { value: { valueSetFromString: vi.fn() } },
|
column$: {
|
||||||
|
value: {
|
||||||
|
valueSetFromString: vi.fn(),
|
||||||
|
type$: { value: 'text' },
|
||||||
|
},
|
||||||
|
},
|
||||||
};
|
};
|
||||||
selectionController.getCellContainer.mockReturnValue(cell);
|
selectionController.getCellContainer.mockReturnValue(cell);
|
||||||
selectionController.selection = TableViewAreaSelection.create({
|
selectionController.selection = TableViewAreaSelection.create({
|
||||||
@@ -117,4 +167,41 @@ describe('Virtual TableHotkeysController', () => {
|
|||||||
expect(selectionController.selection.isEditing).toBe(true);
|
expect(selectionController.selection.isEditing).toBe(true);
|
||||||
expect(evt.preventDefault).toHaveBeenCalled();
|
expect(evt.preventDefault).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each(TAG_COLUMN_TYPES)(
|
||||||
|
'stages draft for %s column instead of valueSetFromString',
|
||||||
|
columnType => {
|
||||||
|
const { logic, selectionController } = createLogic();
|
||||||
|
const ctrl = new VirtualHotkeysController(logic as any);
|
||||||
|
ctrl.hostConnected();
|
||||||
|
const setTagDraft = vi.fn();
|
||||||
|
const cell = {
|
||||||
|
rowId: 'r1',
|
||||||
|
dataset: { rowId: 'r1', columnId: 'c1' },
|
||||||
|
column$: {
|
||||||
|
value: {
|
||||||
|
valueSetFromString: vi.fn(),
|
||||||
|
type$: { value: columnType },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setTagDraft,
|
||||||
|
};
|
||||||
|
selectionController.getCellContainer.mockReturnValue(cell);
|
||||||
|
selectionController.selection = TableViewAreaSelection.create({
|
||||||
|
focus: { rowIndex: 1, columnIndex: 0 },
|
||||||
|
isEditing: false,
|
||||||
|
});
|
||||||
|
const evt = {
|
||||||
|
key: 'C',
|
||||||
|
metaKey: false,
|
||||||
|
ctrlKey: false,
|
||||||
|
altKey: false,
|
||||||
|
preventDefault: vi.fn(),
|
||||||
|
};
|
||||||
|
logic.keyDown({ get: () => ({ raw: evt }) });
|
||||||
|
expect(cell.column$.value.valueSetFromString).not.toHaveBeenCalled();
|
||||||
|
expect(setTagDraft).toHaveBeenCalledWith('C');
|
||||||
|
expect(selectionController.selection.isEditing).toBe(true);
|
||||||
|
}
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -69,8 +69,20 @@ export type TagManagerOptions = {
|
|||||||
options: ReadonlySignal<SelectTag[]>;
|
options: ReadonlySignal<SelectTag[]>;
|
||||||
onOptionsChange: (options: SelectTag[]) => void;
|
onOptionsChange: (options: SelectTag[]) => void;
|
||||||
onComplete?: () => void;
|
onComplete?: () => void;
|
||||||
|
initialDraftText?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// parent elements that can consume tag draft
|
||||||
|
const TABLE_CELL_HOST_SELECTOR =
|
||||||
|
'dv-table-view-cell-container, affine-database-virtual-cell-container';
|
||||||
|
|
||||||
|
export function consumeTagDraftFromTableCellHost(
|
||||||
|
fromElement: Element
|
||||||
|
): string | undefined {
|
||||||
|
const host = fromElement.closest(TABLE_CELL_HOST_SELECTOR) as any;
|
||||||
|
return host?.consumeTagDraft?.();
|
||||||
|
}
|
||||||
|
|
||||||
class TagManager {
|
class TagManager {
|
||||||
changeTag = (option: Partial<SelectTag>) => {
|
changeTag = (option: Partial<SelectTag>) => {
|
||||||
this.ops.onOptionsChange(
|
this.ops.onOptionsChange(
|
||||||
@@ -427,6 +439,15 @@ export class MultiTagSelect extends SignalWatcher(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override connectedCallback() {
|
||||||
|
super.connectedCallback();
|
||||||
|
const draft = this.initialDraftText;
|
||||||
|
if (draft != null && draft !== '') {
|
||||||
|
this.tagManager.text$.value = draft;
|
||||||
|
this.initialDraftText = undefined;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
protected override firstUpdated() {
|
protected override firstUpdated() {
|
||||||
const disposables = this.disposables;
|
const disposables = this.disposables;
|
||||||
this.classList.add(tagSelectContainerStyle);
|
this.classList.add(tagSelectContainerStyle);
|
||||||
@@ -471,6 +492,9 @@ export class MultiTagSelect extends SignalWatcher(
|
|||||||
|
|
||||||
@property({ attribute: false })
|
@property({ attribute: false })
|
||||||
accessor value!: ReadonlySignal<string[]>;
|
accessor value!: ReadonlySignal<string[]>;
|
||||||
|
|
||||||
|
@property({ attribute: false })
|
||||||
|
accessor initialDraftText: string | undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
declare global {
|
declare global {
|
||||||
@@ -481,6 +505,9 @@ declare global {
|
|||||||
|
|
||||||
const popMobileTagSelect = (target: PopupTarget, ops: TagSelectOptions) => {
|
const popMobileTagSelect = (target: PopupTarget, ops: TagSelectOptions) => {
|
||||||
const tagManager = new TagManager(ops);
|
const tagManager = new TagManager(ops);
|
||||||
|
if (ops.initialDraftText) {
|
||||||
|
tagManager.text$.value = ops.initialDraftText;
|
||||||
|
}
|
||||||
const onInput = (e: InputEvent) => {
|
const onInput = (e: InputEvent) => {
|
||||||
tagManager.text$.value = (e.target as HTMLInputElement).value;
|
tagManager.text$.value = (e.target as HTMLInputElement).value;
|
||||||
};
|
};
|
||||||
@@ -604,6 +631,7 @@ export const popTagSelect = (target: PopupTarget, ops: TagSelectOptions) => {
|
|||||||
component.onChange = ops.onChange;
|
component.onChange = ops.onChange;
|
||||||
component.options = ops.options;
|
component.options = ops.options;
|
||||||
component.onOptionsChange = ops.onOptionsChange;
|
component.onOptionsChange = ops.onOptionsChange;
|
||||||
|
component.initialDraftText = ops.initialDraftText;
|
||||||
component.onComplete = () => {
|
component.onComplete = () => {
|
||||||
ops.onComplete?.();
|
ops.onComplete?.();
|
||||||
remove();
|
remove();
|
||||||
|
|||||||
@@ -2,7 +2,10 @@ import { popupTargetFromElement } from '@blocksuite/affine-components/context-me
|
|||||||
import { computed } from '@preact/signals-core';
|
import { computed } from '@preact/signals-core';
|
||||||
import { html } from 'lit/static-html.js';
|
import { html } from 'lit/static-html.js';
|
||||||
|
|
||||||
import { popTagSelect } from '../../core/component/tags/multi-tag-select.js';
|
import {
|
||||||
|
consumeTagDraftFromTableCellHost,
|
||||||
|
popTagSelect,
|
||||||
|
} from '../../core/component/tags/multi-tag-select.js';
|
||||||
import type { SelectTag } from '../../core/index.js';
|
import type { SelectTag } from '../../core/index.js';
|
||||||
import { BaseCellRenderer } from '../../core/property/index.js';
|
import { BaseCellRenderer } from '../../core/property/index.js';
|
||||||
import { createFromBaseCellRenderer } from '../../core/property/renderer.js';
|
import { createFromBaseCellRenderer } from '../../core/property/renderer.js';
|
||||||
@@ -19,6 +22,7 @@ export class MultiSelectCell extends BaseCellRenderer<
|
|||||||
> {
|
> {
|
||||||
closePopup?: () => void;
|
closePopup?: () => void;
|
||||||
private readonly popTagSelect = () => {
|
private readonly popTagSelect = () => {
|
||||||
|
const initialDraftText = consumeTagDraftFromTableCellHost(this);
|
||||||
this.closePopup = popTagSelect(popupTargetFromElement(this), {
|
this.closePopup = popTagSelect(popupTargetFromElement(this), {
|
||||||
name: this.cell.property.name$.value,
|
name: this.cell.property.name$.value,
|
||||||
options: this.options$,
|
options: this.options$,
|
||||||
@@ -29,6 +33,7 @@ export class MultiSelectCell extends BaseCellRenderer<
|
|||||||
},
|
},
|
||||||
onComplete: this._editComplete,
|
onComplete: this._editComplete,
|
||||||
minWidth: 400,
|
minWidth: 400,
|
||||||
|
initialDraftText,
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -2,7 +2,10 @@ import { popupTargetFromElement } from '@blocksuite/affine-components/context-me
|
|||||||
import { computed } from '@preact/signals-core';
|
import { computed } from '@preact/signals-core';
|
||||||
import { html } from 'lit/static-html.js';
|
import { html } from 'lit/static-html.js';
|
||||||
|
|
||||||
import { popTagSelect } from '../../core/component/tags/multi-tag-select.js';
|
import {
|
||||||
|
consumeTagDraftFromTableCellHost,
|
||||||
|
popTagSelect,
|
||||||
|
} from '../../core/component/tags/multi-tag-select.js';
|
||||||
import type { SelectTag } from '../../core/index.js';
|
import type { SelectTag } from '../../core/index.js';
|
||||||
import { BaseCellRenderer } from '../../core/property/index.js';
|
import { BaseCellRenderer } from '../../core/property/index.js';
|
||||||
import { createFromBaseCellRenderer } from '../../core/property/renderer.js';
|
import { createFromBaseCellRenderer } from '../../core/property/renderer.js';
|
||||||
@@ -20,6 +23,7 @@ export class SelectCell extends BaseCellRenderer<
|
|||||||
> {
|
> {
|
||||||
closePopup?: () => void;
|
closePopup?: () => void;
|
||||||
private readonly popTagSelect = () => {
|
private readonly popTagSelect = () => {
|
||||||
|
const initialDraftText = consumeTagDraftFromTableCellHost(this);
|
||||||
this.closePopup = popTagSelect(popupTargetFromElement(this), {
|
this.closePopup = popTagSelect(popupTargetFromElement(this), {
|
||||||
name: this.cell.property.name$.value,
|
name: this.cell.property.name$.value,
|
||||||
mode: 'single',
|
mode: 'single',
|
||||||
@@ -31,6 +35,7 @@ export class SelectCell extends BaseCellRenderer<
|
|||||||
},
|
},
|
||||||
onComplete: this._editComplete,
|
onComplete: this._editComplete,
|
||||||
minWidth: 400,
|
minWidth: 400,
|
||||||
|
initialDraftText,
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -181,6 +181,19 @@ export class DatabaseCellContainer extends SignalWatcher(
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private _tagDraft: string | undefined;
|
||||||
|
|
||||||
|
setTagDraft(value: string) {
|
||||||
|
this._tagDraft = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
consumeTagDraft(): string | undefined {
|
||||||
|
const value = this._tagDraft;
|
||||||
|
this._tagDraft = undefined;
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
isEditing$ = signal(false);
|
isEditing$ = signal(false);
|
||||||
|
|
||||||
rowIndex$ = computed(() => {
|
rowIndex$ = computed(() => {
|
||||||
|
|||||||
@@ -46,6 +46,18 @@ export class TableViewCellContainer extends SignalWatcher(
|
|||||||
@property({ attribute: false })
|
@property({ attribute: false })
|
||||||
accessor rowId!: string;
|
accessor rowId!: string;
|
||||||
|
|
||||||
|
private _tagDraft: string | undefined;
|
||||||
|
|
||||||
|
setTagDraft(value: string) {
|
||||||
|
this._tagDraft = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
consumeTagDraft(): string | undefined {
|
||||||
|
const value = this._tagDraft;
|
||||||
|
this._tagDraft = undefined;
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
cell$ = computed(() => {
|
cell$ = computed(() => {
|
||||||
return this.column.cellGetOrCreate(this.rowId);
|
return this.column.cellGetOrCreate(this.rowId);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,13 +1,26 @@
|
|||||||
|
import type { ReadonlySignal } from '@preact/signals-core';
|
||||||
|
|
||||||
|
import { multiSelectPropertyType } from '../../property-presets/multi-select/define.js';
|
||||||
|
import { selectPropertyType } from '../../property-presets/select/define.js';
|
||||||
import type { TableViewSelectionWithType } from './selection';
|
import type { TableViewSelectionWithType } from './selection';
|
||||||
import { TableViewRowSelection } from './selection';
|
import { TableViewRowSelection } from './selection';
|
||||||
|
|
||||||
export interface TableCell {
|
export interface TableCell {
|
||||||
rowId: string;
|
rowId: string;
|
||||||
|
setTagDraft?(value: string): void;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type ColumnAccessor<T extends TableCell> = (
|
const TAG_COLUMN_TYPES = new Set<string>([
|
||||||
cell: T
|
selectPropertyType.type,
|
||||||
) => { valueSetFromString(rowId: string, value: string): void } | undefined;
|
multiSelectPropertyType.type,
|
||||||
|
]);
|
||||||
|
|
||||||
|
export type ColumnAccessor<T extends TableCell> = (cell: T) =>
|
||||||
|
| {
|
||||||
|
valueSetFromString(rowId: string, value: string): void;
|
||||||
|
type$: ReadonlySignal<string>;
|
||||||
|
}
|
||||||
|
| undefined;
|
||||||
|
|
||||||
export interface StartEditOptions<T extends TableCell> {
|
export interface StartEditOptions<T extends TableCell> {
|
||||||
event: KeyboardEvent;
|
event: KeyboardEvent;
|
||||||
@@ -48,7 +61,13 @@ export function handleCharStartEdit<T extends TableCell>(
|
|||||||
);
|
);
|
||||||
if (cell) {
|
if (cell) {
|
||||||
const column = getColumn(cell);
|
const column = getColumn(cell);
|
||||||
column?.valueSetFromString(cell.rowId, event.key);
|
if (column) {
|
||||||
|
if (TAG_COLUMN_TYPES.has(column.type$.value) && cell.setTagDraft) {
|
||||||
|
cell.setTagDraft(event.key);
|
||||||
|
} else {
|
||||||
|
column.valueSetFromString(cell.rowId, event.key);
|
||||||
|
}
|
||||||
|
}
|
||||||
updateSelection({ ...selection, isEditing: true });
|
updateSelection({ ...selection, isEditing: true });
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
Reference in New Issue
Block a user