mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-08-02 01:49:51 +08:00
fix(editor): restore grouped manual ordering for kanban and arrange (#14630)
Fixes #14531, where mannual vertical order is broken [Root Cause](https://github.com/toeverything/AFFiNE/issues/14531#issuecomment-4052422436) - Restored manual row/card sorting when building grouped kanban data. - Reapplied `sortRow(...)` to each group before rendering `group.rows`. - Fixed group/board arrange to reorder from the full group list, including hidden or empty groups. - Preserved consistent ordering between the settings panel and persisted `groupProperties`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Consolidated internal grouping and sorting logic to ensure consistent ordering across grouped views; visible behavior unchanged. * Moving groups or cards now uses a single, consistent ordering approach to avoid intermittent ordering differences. * **Tests** * Added tests to verify manual per-group card order is applied and preserved when moving cards between groups. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: DarkSky <darksky2048@gmail.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import type { DataSource } from '../core/data-source/base.js';
|
|||||||
import { DetailSelection } from '../core/detail/selection.js';
|
import { DetailSelection } from '../core/detail/selection.js';
|
||||||
import type { FilterGroup } from '../core/filter/types.js';
|
import type { FilterGroup } from '../core/filter/types.js';
|
||||||
import { groupByMatchers } from '../core/group-by/define.js';
|
import { groupByMatchers } from '../core/group-by/define.js';
|
||||||
|
import { GroupTrait, sortByManually } from '../core/group-by/trait.js';
|
||||||
import { t } from '../core/logical/type-presets.js';
|
import { t } from '../core/logical/type-presets.js';
|
||||||
import type { DataViewCellLifeCycle } from '../core/property/index.js';
|
import type { DataViewCellLifeCycle } from '../core/property/index.js';
|
||||||
import { checkboxPropertyModelConfig } from '../property-presets/checkbox/define.js';
|
import { checkboxPropertyModelConfig } from '../property-presets/checkbox/define.js';
|
||||||
@@ -25,6 +26,7 @@ import {
|
|||||||
import type { KanbanCard } from '../view-presets/kanban/pc/card.js';
|
import type { KanbanCard } from '../view-presets/kanban/pc/card.js';
|
||||||
import { KanbanDragController } from '../view-presets/kanban/pc/controller/drag.js';
|
import { KanbanDragController } from '../view-presets/kanban/pc/controller/drag.js';
|
||||||
import type { KanbanGroup } from '../view-presets/kanban/pc/group.js';
|
import type { KanbanGroup } from '../view-presets/kanban/pc/group.js';
|
||||||
|
import type { Row } from '../core/view-manager/row.js';
|
||||||
|
|
||||||
type Column = {
|
type Column = {
|
||||||
id: string;
|
id: string;
|
||||||
@@ -214,7 +216,205 @@ const createDragController = () => {
|
|||||||
return new KanbanDragController({} as DragLogic);
|
return new KanbanDragController({} as DragLogic);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const createTestRow = (rowId: string): Row => ({
|
||||||
|
rowId,
|
||||||
|
cells$: signal([]) as Row['cells$'],
|
||||||
|
index$: signal<Row['index$']['value']>(undefined),
|
||||||
|
prev$: signal<Row | undefined>(undefined),
|
||||||
|
next$: signal<Row | undefined>(undefined),
|
||||||
|
delete: vi.fn(),
|
||||||
|
move: vi.fn(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const createGroupTraitHarness = (options?: {
|
||||||
|
groupProperties?: Array<{
|
||||||
|
key: string;
|
||||||
|
hide: boolean;
|
||||||
|
manuallyCardSort: string[];
|
||||||
|
}>;
|
||||||
|
rowIds?: string[];
|
||||||
|
values?: Record<string, boolean>;
|
||||||
|
}) => {
|
||||||
|
const dataSource = createMockDataSource([
|
||||||
|
{
|
||||||
|
id: 'checkbox',
|
||||||
|
type: checkboxPropertyModelConfig.type,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const property = {
|
||||||
|
id: 'checkbox',
|
||||||
|
dataType$: signal(t.boolean.instance()),
|
||||||
|
meta$: signal({ config: {} }),
|
||||||
|
};
|
||||||
|
|
||||||
|
const groupProperties = options?.groupProperties ?? [
|
||||||
|
{
|
||||||
|
key: 'true',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: [],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: 'false',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: [],
|
||||||
|
},
|
||||||
|
];
|
||||||
|
const rows = options?.rowIds ?? [];
|
||||||
|
const data$ = signal({
|
||||||
|
groupProperties,
|
||||||
|
});
|
||||||
|
const cellValues = new Map(
|
||||||
|
Object.entries(options?.values ?? {}).map(([rowId, value]) => [
|
||||||
|
`${rowId}:checkbox`,
|
||||||
|
value,
|
||||||
|
])
|
||||||
|
);
|
||||||
|
const cells = new Map<
|
||||||
|
string,
|
||||||
|
{
|
||||||
|
jsonValue$: ReturnType<typeof signal<boolean | undefined>>;
|
||||||
|
jsonValueSet: ReturnType<typeof vi.fn<(value: unknown) => void>>;
|
||||||
|
valueSet: ReturnType<typeof vi.fn<(value: unknown) => void>>;
|
||||||
|
}
|
||||||
|
>();
|
||||||
|
|
||||||
|
const cellGetOrCreate = (rowId: string, propertyId: string) => {
|
||||||
|
const key = `${rowId}:${propertyId}`;
|
||||||
|
const existing = cells.get(key);
|
||||||
|
if (existing) {
|
||||||
|
return existing;
|
||||||
|
}
|
||||||
|
|
||||||
|
const jsonValue$ = signal(cellValues.get(key));
|
||||||
|
const update = (value: unknown) => {
|
||||||
|
jsonValue$.value = value as boolean | undefined;
|
||||||
|
cellValues.set(key, value as boolean);
|
||||||
|
};
|
||||||
|
const cell = {
|
||||||
|
jsonValue$,
|
||||||
|
jsonValueSet: vi.fn(update),
|
||||||
|
valueSet: vi.fn(update),
|
||||||
|
};
|
||||||
|
cells.set(key, cell);
|
||||||
|
return cell;
|
||||||
|
};
|
||||||
|
|
||||||
|
const view = {
|
||||||
|
data$,
|
||||||
|
rows$: signal(rows.map(createTestRow)),
|
||||||
|
isLocked$: signal(false),
|
||||||
|
manager: {
|
||||||
|
dataSource: asDataSource(dataSource),
|
||||||
|
},
|
||||||
|
propertyGetOrCreate: () => property,
|
||||||
|
cellGetOrCreate,
|
||||||
|
};
|
||||||
|
|
||||||
|
const groupBy$ = signal<GroupBy | undefined>({
|
||||||
|
type: 'groupBy',
|
||||||
|
columnId: 'checkbox',
|
||||||
|
name: 'boolean',
|
||||||
|
hideEmpty: false,
|
||||||
|
sort: { desc: false },
|
||||||
|
});
|
||||||
|
|
||||||
|
const ops = {
|
||||||
|
groupBySet: vi.fn(),
|
||||||
|
sortGroup: (keys: string[], asc?: boolean) => {
|
||||||
|
const sorted = sortByManually(
|
||||||
|
keys,
|
||||||
|
value => value,
|
||||||
|
data$.value.groupProperties.map(value => value.key)
|
||||||
|
);
|
||||||
|
return asc === false ? sorted.reverse() : sorted;
|
||||||
|
},
|
||||||
|
sortRow: (groupKey: string, groupedRows: Row[]) => {
|
||||||
|
const group = data$.value.groupProperties.find(
|
||||||
|
value => value.key === groupKey
|
||||||
|
);
|
||||||
|
return sortByManually(
|
||||||
|
groupedRows,
|
||||||
|
row => row.rowId,
|
||||||
|
group?.manuallyCardSort ?? []
|
||||||
|
);
|
||||||
|
},
|
||||||
|
changeGroupSort: vi.fn(),
|
||||||
|
changeRowSort: vi.fn(),
|
||||||
|
changeGroupHide: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
return {
|
||||||
|
groupTrait: new GroupTrait(groupBy$, view as never, ops),
|
||||||
|
ops,
|
||||||
|
cells,
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
describe('kanban', () => {
|
describe('kanban', () => {
|
||||||
|
describe('group trait', () => {
|
||||||
|
it('reapplies manual card order when building grouped rows', () => {
|
||||||
|
const { groupTrait } = createGroupTraitHarness({
|
||||||
|
groupProperties: [
|
||||||
|
{
|
||||||
|
key: 'true',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: ['row-2', 'row-1'],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: 'false',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: [],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
rowIds: ['row-1', 'row-2'],
|
||||||
|
values: {
|
||||||
|
'row-1': true,
|
||||||
|
'row-2': true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
groupTrait.groupsDataList$.value
|
||||||
|
?.find(group => group.key === 'true')
|
||||||
|
?.rows.map(row => row.rowId)
|
||||||
|
).toEqual(['row-2', 'row-1']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('preserves manual group order when updating card sort', () => {
|
||||||
|
const { groupTrait, ops, cells } = createGroupTraitHarness({
|
||||||
|
groupProperties: [
|
||||||
|
{
|
||||||
|
key: 'false',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: ['row-1'],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: 'true',
|
||||||
|
hide: false,
|
||||||
|
manuallyCardSort: ['row-2'],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
rowIds: ['row-1', 'row-2'],
|
||||||
|
values: {
|
||||||
|
'row-1': false,
|
||||||
|
'row-2': true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
groupTrait.moveCardTo('row-1', 'false', 'true', 'end');
|
||||||
|
|
||||||
|
expect(ops.changeRowSort).toHaveBeenCalledWith(
|
||||||
|
['false', 'true'],
|
||||||
|
'true',
|
||||||
|
['row-2', 'row-1']
|
||||||
|
);
|
||||||
|
expect(cells.get('row-1:checkbox')?.jsonValueSet).toHaveBeenCalledWith(
|
||||||
|
true
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('group-by define', () => {
|
describe('group-by define', () => {
|
||||||
it('boolean group should not include ungroup bucket', () => {
|
it('boolean group should not include ungroup bucket', () => {
|
||||||
const booleanGroup = groupByMatchers.find(
|
const booleanGroup = groupByMatchers.find(
|
||||||
|
|||||||
@@ -19,7 +19,6 @@ import type { SingleView } from '../view-manager/single-view.js';
|
|||||||
import { compareDateKeys } from './compare-date-keys.js';
|
import { compareDateKeys } from './compare-date-keys.js';
|
||||||
import { defaultGroupBy } from './default.js';
|
import { defaultGroupBy } from './default.js';
|
||||||
import { findGroupByConfigByName, getGroupByService } from './matcher.js';
|
import { findGroupByConfigByName, getGroupByService } from './matcher.js';
|
||||||
// Test
|
|
||||||
import type { GroupByConfig } from './types.js';
|
import type { GroupByConfig } from './types.js';
|
||||||
|
|
||||||
export type GroupInfo<
|
export type GroupInfo<
|
||||||
@@ -88,6 +87,60 @@ function hasGroupProperties(
|
|||||||
return value === undefined || Array.isArray(value);
|
return value === undefined || Array.isArray(value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const getOrderedGroupKeys = (
|
||||||
|
keys: string[],
|
||||||
|
groupInfo: GroupInfo | undefined,
|
||||||
|
sortGroup: (keys: string[], asc?: boolean) => string[],
|
||||||
|
sortAsc: boolean
|
||||||
|
) => {
|
||||||
|
if (groupInfo?.config.matchType.name === 'Date') {
|
||||||
|
return [...keys].sort(compareDateKeys(groupInfo.config.name, sortAsc));
|
||||||
|
}
|
||||||
|
return sortGroup(keys, sortAsc);
|
||||||
|
};
|
||||||
|
|
||||||
|
const applyGroupRowSort = <T extends { rows: Row[] }>(
|
||||||
|
groups: Record<string, T>,
|
||||||
|
orderedKeys: string[],
|
||||||
|
sortRow: (groupKey: string, rows: Row[]) => Row[]
|
||||||
|
) => {
|
||||||
|
orderedKeys.forEach(key => {
|
||||||
|
const group = groups[key];
|
||||||
|
if (!group) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
group.rows = sortRow(key, group.rows);
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
const reorderGroupKeys = (
|
||||||
|
keys: string[],
|
||||||
|
groupKey: string,
|
||||||
|
position: InsertToPosition
|
||||||
|
) => {
|
||||||
|
const currentIndex = keys.findIndex(key => key === groupKey);
|
||||||
|
if (currentIndex < 0) {
|
||||||
|
return keys;
|
||||||
|
}
|
||||||
|
if (typeof position === 'object') {
|
||||||
|
if (position.id === groupKey) {
|
||||||
|
return keys;
|
||||||
|
}
|
||||||
|
if (!keys.includes(position.id)) {
|
||||||
|
return keys;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const reordered = [...keys];
|
||||||
|
reordered.splice(currentIndex, 1);
|
||||||
|
const index = insertPositionToIndex(position, reordered, key => key);
|
||||||
|
if (index < 0) {
|
||||||
|
return keys;
|
||||||
|
}
|
||||||
|
reordered.splice(index, 0, groupKey);
|
||||||
|
return reordered;
|
||||||
|
};
|
||||||
|
|
||||||
export class GroupTrait {
|
export class GroupTrait {
|
||||||
hideEmpty$ = signal<boolean>(true);
|
hideEmpty$ = signal<boolean>(true);
|
||||||
sortAsc$ = signal<boolean>(true);
|
sortAsc$ = signal<boolean>(true);
|
||||||
@@ -202,15 +255,13 @@ export class GroupTrait {
|
|||||||
if (!map) return;
|
if (!map) return;
|
||||||
|
|
||||||
const gi = this.groupInfo$.value;
|
const gi = this.groupInfo$.value;
|
||||||
let ordered: string[];
|
const ordered = getOrderedGroupKeys(
|
||||||
|
Object.keys(map),
|
||||||
if (gi?.config.matchType.name === 'Date') {
|
gi,
|
||||||
ordered = Object.keys(map).sort(
|
this.ops.sortGroup,
|
||||||
compareDateKeys(gi.config.name, this.sortAsc$.value)
|
this.sortAsc$.value
|
||||||
);
|
);
|
||||||
} else {
|
applyGroupRowSort(map, ordered, this.ops.sortRow);
|
||||||
ordered = this.ops.sortGroup(Object.keys(map), this.sortAsc$.value);
|
|
||||||
}
|
|
||||||
|
|
||||||
return ordered
|
return ordered
|
||||||
.map(k => map[k])
|
.map(k => map[k])
|
||||||
@@ -233,14 +284,13 @@ export class GroupTrait {
|
|||||||
const info = this.groupInfo$.value;
|
const info = this.groupInfo$.value;
|
||||||
if (!map || !info) return;
|
if (!map || !info) return;
|
||||||
|
|
||||||
let orderedKeys: string[];
|
const orderedKeys = getOrderedGroupKeys(
|
||||||
if (info.config.matchType.name === 'Date') {
|
Object.keys(map),
|
||||||
orderedKeys = Object.keys(map).sort(
|
info,
|
||||||
compareDateKeys(info.config.name, this.sortAsc$.value)
|
this.ops.sortGroup,
|
||||||
);
|
this.sortAsc$.value
|
||||||
} else {
|
);
|
||||||
orderedKeys = this.ops.sortGroup(Object.keys(map), this.sortAsc$.value);
|
applyGroupRowSort(map, orderedKeys, this.ops.sortRow);
|
||||||
}
|
|
||||||
|
|
||||||
const visible: Group[] = [];
|
const visible: Group[] = [];
|
||||||
const hidden: Group[] = [];
|
const hidden: Group[] = [];
|
||||||
@@ -430,23 +480,29 @@ export class GroupTrait {
|
|||||||
.map(row => row.rowId) ?? [];
|
.map(row => row.rowId) ?? [];
|
||||||
const index = insertPositionToIndex(position, rows, row => row);
|
const index = insertPositionToIndex(position, rows, row => row);
|
||||||
rows.splice(index, 0, rowId);
|
rows.splice(index, 0, rowId);
|
||||||
const groupKeys = Object.keys(groupMap);
|
const groupKeys = getOrderedGroupKeys(
|
||||||
|
Object.keys(groupMap),
|
||||||
|
this.groupInfo$.value,
|
||||||
|
this.ops.sortGroup,
|
||||||
|
this.sortAsc$.value
|
||||||
|
);
|
||||||
this.ops.changeRowSort(groupKeys, toGroupKey, rows);
|
this.ops.changeRowSort(groupKeys, toGroupKey, rows);
|
||||||
}
|
}
|
||||||
|
|
||||||
moveGroupTo(groupKey: string, position: InsertToPosition) {
|
moveGroupTo(groupKey: string, position: InsertToPosition) {
|
||||||
const groups = this.groupsDataList$.value;
|
const groups = this.groupsDataListAll$.value;
|
||||||
if (!groups) {
|
if (!groups) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const keys = groups.map(v => v!.key);
|
const currentKeys = groups.map(group => group.key);
|
||||||
keys.splice(
|
const reorderedKeys = reorderGroupKeys(currentKeys, groupKey, position);
|
||||||
keys.findIndex(key => key === groupKey),
|
if (
|
||||||
1
|
currentKeys.length === reorderedKeys.length &&
|
||||||
);
|
currentKeys.every((key, index) => key === reorderedKeys[index])
|
||||||
const index = insertPositionToIndex(position, keys, key => key);
|
) {
|
||||||
keys.splice(index, 0, groupKey);
|
return;
|
||||||
this.changeGroupSort(keys);
|
}
|
||||||
|
this.changeGroupSort(reorderedKeys);
|
||||||
}
|
}
|
||||||
|
|
||||||
removeFromGroup(rowId: string, key: string) {
|
removeFromGroup(rowId: string, key: string) {
|
||||||
|
|||||||
Reference in New Issue
Block a user