Skip to content

Commit 604c7ce

Browse files
tobiutobiu
committed
refactor: Fix Grid/Table Selection Models for internalId (#9072)
1 parent d1c2590 commit 604c7ce

13 files changed

Lines changed: 932 additions & 853 deletions

File tree

‎apps/devrank/resources/tracker.json‎

Lines changed: 372 additions & 447 deletions
Large diffs are not rendered by default.

‎apps/devrank/resources/users.json‎

Lines changed: 372 additions & 372 deletions
Large diffs are not rendered by default.

‎src/grid/Body.mjs‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -745,12 +745,7 @@ class GridBody extends Component {
745745
if (target.data?.field) {
746746
dataField = target.data.field;
747747
recordId = target.data.recordId;
748-
749-
if (!me.useInternalId && me.store.getKeyType()?.startsWith('int')) {
750-
recordId = parseInt(recordId)
751-
}
752-
753-
record = me.store.get(recordId);
748+
record = me.getRecord(recordId);
754749
break
755750
}
756751
}
@@ -775,12 +770,7 @@ class GridBody extends Component {
775770
for (target of data.path) {
776771
if (target.cls?.includes('neo-grid-row') && target.data?.recordId) {
777772
recordId = target.data.recordId;
778-
779-
if (!me.useInternalId && me.store.getKeyType()?.startsWith('int')) {
780-
recordId = parseInt(recordId)
781-
}
782-
783-
record = me.store.get(recordId);
773+
record = me.getRecord(recordId);
784774
break
785775
}
786776
}
@@ -919,6 +909,15 @@ class GridBody extends Component {
919909
return record;
920910
}
921911

912+
// Check if nodeId is a recordId (internalId or PK)
913+
if (me.useInternalId) {
914+
record = me.store.items.find(r => me.store.getInternalId(r) === nodeId);
915+
if (record) return record;
916+
} else {
917+
record = me.store.get(nodeId);
918+
if (record) return record;
919+
}
920+
922921
parentNodes = VDomUtil.getParentNodes(me.vdom, nodeId);
923922

924923
for (node of parentNodes || []) {
@@ -948,7 +947,7 @@ class GridBody extends Component {
948947
let me = this,
949948
dataField = me.getDataField(logicalId),
950949
recordId = logicalId.substring(0, logicalId.length - dataField.length - 2),
951-
record = me.store.get(recordId);
950+
record = me.getRecord(recordId); // Uses the new robust getRecord()
952951

953952
if (!record) {
954953
record = me.store.get(parseInt(recordId))

‎src/selection/grid/BaseModel.mjs‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ class BaseModel extends Model {
5959
let me = this,
6060
{view} = me,
6161
{store} = view,
62-
isCell = me.ntype.includes('cell'),
6362
processed = new Set();
6463

6564
items.forEach(item => {
6665
let hasChanged = false,
66+
isCell = item.toString().includes('__'),
6767
recordId, row;
6868

6969
if (isCell) {
@@ -109,6 +109,25 @@ class BaseModel extends Model {
109109
processed.add(recordId);
110110
let record = store.get(recordId);
111111

112+
// If record not found by PK, check if it's an internalId
113+
if (!record && view.useInternalId) {
114+
// 1. Fast path: Check currently rendered rows
115+
// This covers the 99% case where user clicks a visible row
116+
for (let i = 0; i < view.items.length; i++) {
117+
let r = view.items[i].record;
118+
if (r && view.getRecordId(r) === recordId) {
119+
record = r;
120+
break
121+
}
122+
}
123+
124+
// 2. Slow path: Iteration (only if we need to select an off-screen row by internalId)
125+
// This is rare for internalIds which are mostly UI-driven
126+
/* if (!record) {
127+
// todo: store.findByInternalId(recordId) or similar optimization
128+
} */
129+
}
130+
112131
if (record) {
113132
row = view.getRow(record);
114133
if (row) {
@@ -244,6 +263,36 @@ class BaseModel extends Model {
244263
return null
245264
}
246265

266+
/**
267+
* Resolves a record from an ID (PK or internalId).
268+
* @param {String|Number} id
269+
* @returns {Neo.data.Record|null}
270+
*/
271+
getRowRecord(id) {
272+
if (!id) return null;
273+
274+
let me = this,
275+
{view} = me,
276+
{store}= view,
277+
record = store.get(id);
278+
279+
if (record) return record;
280+
281+
// Fast path: Check visible rows
282+
if (view.items) {
283+
let row = view.items.find(r => r.record && view.getRecordId(r.record) === id);
284+
if (row) return row.record
285+
}
286+
287+
// Slow path: Scan store
288+
if (view.useInternalId) {
289+
record = store.items.find(r => store.getInternalId(r) === id);
290+
if (record) return record
291+
}
292+
293+
return null
294+
}
295+
247296
/**
248297
* @param {Number|String} recordId
249298
* @returns {Neo.grid.Row|null}

‎src/selection/grid/CellColumnModel.mjs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,18 @@ class CellColumnModel extends CellModel {
3535
let me = this,
3636
{view} = me,
3737
cellId = data.data.currentTarget,
38-
dataField = cellId && view.getDataField(cellId);
38+
dataField = cellId && view.getDataField(cellId),
39+
newSelection;
3940

4041
if (dataField) {
41-
me.selectedColumns = me.isSelected(cellId) ? [] : [dataField];
42-
view.createViewData(true)
42+
newSelection = me.isSelected(cellId) ? [] : [dataField];
43+
44+
if (!Neo.isEqual(me.selectedColumns, newSelection)) {
45+
me.selectedColumns = newSelection;
46+
view.createViewData() // Flush
47+
} else {
48+
view.createViewData(true) // Silent
49+
}
4350
}
4451

4552
super.onCellClick(data)
@@ -67,7 +74,7 @@ class CellColumnModel extends CellModel {
6774

6875
me.selectedColumns = [dataFields[index]];
6976

70-
view.createViewData(true);
77+
view.createViewData();
7178

7279
super.onNavKeyColumn(step)
7380
}

‎src/selection/grid/CellColumnRowModel.mjs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,18 @@ class CellColumnRowModel extends CellRowModel {
3535
let me = this,
3636
{view} = me,
3737
{dataField, record} = data,
38-
logicalId;
38+
logicalId, newSelection;
3939

4040
if (dataField && record) {
41-
logicalId = view.getLogicalCellId(record, dataField);
42-
me.selectedColumns = me.isSelected(logicalId) ? [] : [dataField];
43-
view.createViewData(true)
41+
logicalId = view.getLogicalCellId(record, dataField);
42+
newSelection = me.isSelected(logicalId) ? [] : [dataField];
43+
44+
if (!Neo.isEqual(me.selectedColumns, newSelection)) {
45+
me.selectedColumns = newSelection;
46+
view.createViewData() // Flush
47+
} else {
48+
view.createViewData(true) // Silent
49+
}
4450
}
4551

4652
super.onCellClick(data)
@@ -68,7 +74,7 @@ class CellColumnRowModel extends CellRowModel {
6874

6975
me.selectedColumns = [dataFields[index]];
7076

71-
view.createViewData(true);
77+
view.createViewData();
7278

7379
super.onNavKeyColumn(step)
7480
}

‎src/selection/grid/CellModel.mjs‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,37 @@ class CellModel extends BaseModel {
4343
super.destroy(...args)
4444
}
4545

46+
/**
47+
* Resolves a record from a logical cell ID.
48+
* @param {String} logicalId
49+
* @returns {Neo.data.Record|null}
50+
*/
51+
getRecord(logicalId) {
52+
let me = this,
53+
{view} = me,
54+
{store} = view,
55+
dataField = view.getDataField(logicalId),
56+
// logicalId format: recordId__dataField
57+
recordId = logicalId.substring(0, logicalId.length - dataField.length - 2),
58+
record = store.get(recordId);
59+
60+
if (record) return record;
61+
62+
// Fast path: Check visible rows
63+
if (view.items) {
64+
let row = view.items.find(r => view.getRecordId(r.record) === recordId);
65+
if (row) return row.record
66+
}
67+
68+
// Slow path: Scan store
69+
if (view.useInternalId) {
70+
record = store.items.find(r => store.getInternalId(r) === recordId);
71+
if (record) return record
72+
}
73+
74+
return null
75+
}
76+
4677
/**
4778
* @param {Object} data
4879
*/
@@ -95,7 +126,7 @@ class CellModel extends BaseModel {
95126

96127
if (me.hasSelection()) {
97128
currentColumn = view.getDataField(me.items[0]);
98-
record = view.getRecordFromLogicalId(me.items[0])
129+
record = me.getRecord(me.items[0])
99130
} else {
100131
currentColumn = dataFields[0];
101132
record = store.getAt(0)
@@ -129,7 +160,7 @@ class CellModel extends BaseModel {
129160
dataField, newIndex;
130161

131162
if (me.hasSelection()) {
132-
currentIndex = store.indexOf(view.getRecordFromLogicalId(me.items[0]));
163+
currentIndex = store.indexOf(me.getRecord(me.items[0]));
133164
dataField = view.getDataField(me.items[0])
134165
} else {
135166
dataField = me.dataFields[0]

‎src/selection/grid/CellRowModel.mjs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class CellRowModel extends CellModel {
3838
if (me.hasAnnotations(record)) {
3939
me.updateAnnotations(record)
4040
} else {
41-
me[me.isSelected(logicalId) ? 'deselectRow' : 'selectRow'](view.store.getKey(record), true)
41+
me[me.isSelected(logicalId) ? 'deselectRow' : 'selectRow'](view.getRecordId(record), true)
4242
}
4343
}
4444

@@ -53,8 +53,8 @@ class CellRowModel extends CellModel {
5353
{view} = me,
5454
{store} = view,
5555
countRecords = store.getCount(),
56-
recordId = me.selectedRows[0] || store.getKey(store.getAt(0)),
57-
record = store.get(recordId),
56+
recordId = me.selectedRows[0] || view.getRecordId(store.getAt(0)),
57+
record = me.getRowRecord(recordId),
5858
index = store.indexOf(record),
5959
newIndex = (index + step) % countRecords;
6060

@@ -67,7 +67,7 @@ class CellRowModel extends CellModel {
6767
if (me.hasAnnotations(record)) {
6868
me.updateAnnotations(record)
6969
} else {
70-
recordId = store.getKey(record);
70+
recordId = view.getRecordId(record);
7171

7272
if (recordId) {
7373
me.selectRow(recordId, true) // silent

‎src/selection/grid/RowModel.mjs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ class RowModel extends BaseModel {
6565
{view} = me,
6666
{store} = view,
6767
countRecords = store.getCount(),
68-
recordId = me.selectedRows[0] || store.getKey(store.getAt(0)),
69-
record = store.get(recordId),
68+
recordId = me.selectedRows[0] || view.getRecordId(store.getAt(0)),
69+
record = me.getRowRecord(recordId),
7070
index = store.indexOf(record),
7171
newIndex = (index + step) % countRecords;
7272

‎src/selection/table/BaseModel.mjs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,33 @@ class BaseModel extends Model {
2222
get dataFields() {
2323
return this.view.parent.columns.map(column => column.dataField)
2424
}
25+
26+
/**
27+
* Resolves a record from an ID (PK or internalId).
28+
* @param {String|Number} id
29+
* @returns {Neo.data.Record|null}
30+
*/
31+
getRowRecord(id) {
32+
if (!id) return null;
33+
34+
let me = this,
35+
{view} = me,
36+
{store}= view,
37+
record = store.get(id);
38+
39+
if (record) return record;
40+
41+
// Table Body creates rows directly in vdom, not as components in items (mostly).
42+
// But getRecordByRowId might help if we have the row ID.
43+
// For internalId we need to check store.
44+
45+
if (view.useInternalId) {
46+
record = store.items.find(r => store.getInternalId(r) === id);
47+
if (record) return record
48+
}
49+
50+
return null
51+
}
2552
}
2653

2754
export default Neo.setupClass(BaseModel);

0 commit comments

Comments
 (0)