Skip to content

Commit e764a06

Browse files
committed
Refactor column management and UI fixes
Make column label checks synchronous and simplify ID/label handling across create-column and manage-columns. Key changes: - Remove unnecessary async from columnLabelCheck and stop awaiting it. - Use Array.some instead of find to check assigned annotation IDs. - Improve cachedAnnotations check to ensure non-empty arrays. - Generate column IDs without unused label param; keep robust UUID fallback. - Remove legacy id normalization; rely on API response handling. - Fix merge UI index handling (use label index) and stop filtering out empty indexes when merging. - Consider rawLabel when detecting duplicate labels. - Parse response bodies directly (const data = await res.json()) and reset workspace UI (uncheck checkboxes and call handleModeChange) after merge/extend operations to avoid stale state. - When extending locally, map to originalColumnLabels to preserve labels. These edits address correctness issues, avoid stale UI/annotation state, and simplify logic for maintainability.
1 parent 3d8984c commit e764a06

2 files changed

Lines changed: 47 additions & 45 deletions

File tree

components/create-column/index.js

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ class TpenCreateColumn extends HTMLElement {
282282
this.cleanup.run()
283283
}
284284

285-
async columnLabelCheck() {
285+
columnLabelCheck() {
286286
this.originalColumnLabels = this.existingColumns.map(col => col.rawLabel ?? col.label)
287287
this.existingColumns = this.existingColumns.map((col, index) => {
288288
const rawLabel = col.rawLabel ?? col.label
@@ -311,13 +311,13 @@ class TpenCreateColumn extends HTMLElement {
311311
lines: column.lines ?? []
312312
})) || []
313313
const assignedAnnotationIds = []
314-
await this.columnLabelCheck()
314+
this.columnLabelCheck()
315315
this.existingColumns.forEach(column => {
316316
column.lines.forEach(annoId => assignedAnnotationIds.push({
317317
lineId: annoId, columnLabel: column.label
318318
}))
319319
})
320-
this.totalIds = annotations.filter(anno => !assignedAnnotationIds.find(a => a.lineId === anno.lineId)).map(a => a.lineId)
320+
this.totalIds = annotations.filter(anno => !assignedAnnotationIds.some(a => a.lineId === anno.lineId)).map(a => a.lineId)
321321
localStorage.setItem('annotationsState', JSON.stringify({
322322
remainingIDs: this.totalIds,
323323
selectedIDs: []
@@ -336,7 +336,7 @@ class TpenCreateColumn extends HTMLElement {
336336
*/
337337
async reloadColumns() {
338338
try {
339-
await this.columnLabelCheck()
339+
this.columnLabelCheck()
340340
// Always ensure TPEN.activeProject is source of truth
341341
this.syncProjectColumnsFromExisting()
342342
this.selectedBoxes = []
@@ -366,7 +366,7 @@ class TpenCreateColumn extends HTMLElement {
366366
.find(p => p.id.split('/').pop() === this.annotationPageID)
367367
}
368368

369-
generateColumnId(label) {
369+
generateColumnId() {
370370
return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
371371
}
372372

@@ -403,8 +403,6 @@ class TpenCreateColumn extends HTMLElement {
403403
label: rawLabel,
404404
lines: [...new Set(column.lines ?? [])]
405405
}
406-
// Ensure id is consistent
407-
if (!updated.id && updated._id) updated.id = updated._id
408406
return updated
409407
})
410408
}
@@ -421,11 +419,11 @@ class TpenCreateColumn extends HTMLElement {
421419
}
422420

423421
refreshFromCache() {
424-
if (!this.cachedImageInfo || !this.cachedAnnotations) return false
422+
if (!this.cachedImageInfo || !this.cachedAnnotations?.length) return false
425423
const { imgWidth, imgHeight } = this.cachedImageInfo
426424
const assignedAnnotationIds = this.buildAssignedAnnotations()
427425
this.totalIds = this.cachedAnnotations
428-
.filter(anno => !assignedAnnotationIds.find(a => a.lineId === anno.lineId))
426+
.filter(anno => !assignedAnnotationIds.some(a => a.lineId === anno.lineId))
429427
.map(a => a.lineId)
430428
localStorage.setItem('annotationsState', JSON.stringify({
431429
remainingIDs: this.totalIds,
@@ -438,7 +436,7 @@ class TpenCreateColumn extends HTMLElement {
438436
}
439437

440438
mergeColumnsLocal(newLabel, columnIndexes) {
441-
const mergeIndexes = [...new Set(columnIndexes)].filter(i => i !== '').sort((a, b) => b - a)
439+
const mergeIndexes = [...new Set(columnIndexes)].sort((a, b) => b - a)
442440
const mergedLines = []
443441
mergeIndexes.forEach(index => {
444442
const column = this.existingColumns[index]
@@ -558,7 +556,7 @@ class TpenCreateColumn extends HTMLElement {
558556
input.classList.add('merge-column-input')
559557
workspaceToolbar.appendChild(input)
560558

561-
columnLabels.forEach(label => {
559+
columnLabels.forEach((label, labelIndex) => {
562560
const btn = document.createElement('button')
563561
btn.classList.add('merge-label-btn')
564562
btn.textContent = label
@@ -570,12 +568,12 @@ class TpenCreateColumn extends HTMLElement {
570568
btn.dataset.selected = 'true'
571569
btn.style.backgroundColor = 'rgb(255, 255, 255)'
572570
btn.style.color = 'var(--primary-color)'
573-
columnLabelsToMerge.push(columnLabels.indexOf(label) !== -1 ? columnLabels.indexOf(label) : '')
571+
columnLabelsToMerge.push(labelIndex)
574572
} else {
575573
delete btn.dataset.selected
576574
btn.style.backgroundColor = 'var(--primary-color)'
577575
btn.style.color = 'rgb(255, 255, 255)'
578-
const index = columnLabelsToMerge.indexOf(columnLabels.indexOf(label))
576+
const index = columnLabelsToMerge.indexOf(labelIndex)
579577
if (index > -1) {
580578
columnLabelsToMerge.splice(index, 1)
581579
}
@@ -599,8 +597,9 @@ class TpenCreateColumn extends HTMLElement {
599597
}
600598

601599
const duplicate = this.existingColumns.some(col => {
602-
const existingLabel = (col.label ?? "").toString().trim()
603-
return existingLabel === newLabel
600+
const displayLabel = (col.label ?? "").toString().trim()
601+
const rawLabel = (col.rawLabel ?? "").toString().trim()
602+
return displayLabel === newLabel || rawLabel === newLabel
604603
})
605604
if (duplicate) {
606605
return TPEN.eventDispatcher.dispatch("tpen-toast", {
@@ -624,12 +623,14 @@ class TpenCreateColumn extends HTMLElement {
624623
TPEN.eventDispatcher.dispatch("tpen-toast", {
625624
status: "success", message: 'Columns merged successfully.'
626625
})
627-
let data = null
628-
try { data = await res.json() } catch {}
626+
const data = await res.json()
629627
if (!this.updateColumnsFromResponse(data)) {
630628
this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
631629
}
632630
await this.reloadColumns()
631+
// Reset workspace UI to avoid stale state
632+
this.mergeColumnsCheckbox.checked = false
633+
this.handleModeChange()
633634
} catch (error) {
634635
TPEN.eventDispatcher.dispatch("tpen-toast", {
635636
status: "error", message: error.message
@@ -725,12 +726,14 @@ class TpenCreateColumn extends HTMLElement {
725726
TPEN.eventDispatcher.dispatch("tpen-toast", {
726727
status: "success", message: 'Column extended successfully.'
727728
})
728-
let data = null
729-
try { data = await res.json() } catch {}
729+
const data = await res.json()
730730
if (!this.updateColumnsFromResponse(data)) {
731-
this.extendColumnLocal(columnToExtend, annotationIdsToAdd)
731+
this.extendColumnLocal(this.originalColumnLabels[columnToExtend], annotationIdsToAdd)
732732
}
733733
await this.reloadColumns()
734+
// Reset workspace UI to avoid stale state
735+
this.extendColumnCheckbox.checked = false
736+
this.handleModeChange()
734737
} catch (error) {
735738
TPEN.eventDispatcher.dispatch("tpen-toast", {
736739
status: "error", message: error.message
@@ -952,8 +955,7 @@ class TpenCreateColumn extends HTMLElement {
952955
TPEN.eventDispatcher.dispatch("tpen-toast", {
953956
status: "success", message: 'Column created successfully.'
954957
})
955-
let data = null
956-
try { data = await res.json() } catch {}
958+
const data = await res.json()
957959
if (!this.updateColumnsFromResponse(data)) {
958960
this.createColumnLocal(columnLabel, selectedIDs)
959961
}
@@ -978,8 +980,7 @@ class TpenCreateColumn extends HTMLElement {
978980
TPEN.eventDispatcher.dispatch("tpen-toast", {
979981
status: "info", message: 'All columns cleared successfully.'
980982
})
981-
let data = null
982-
try { data = await res.json() } catch {}
983+
const data = await res.json()
983984
if (!this.updateColumnsFromResponse(data)) {
984985
this.clearColumnsLocal()
985986
}

interfaces/manage-columns/index.js

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ class TpenManageColumns extends HTMLElement {
352352
this.cleanup.run()
353353
}
354354

355-
async columnLabelCheck() {
355+
columnLabelCheck() {
356356
const AUTO_COLUMN_PREFIX = "Column "
357357
const AUTO_LABEL_ID_LENGTH = 24
358358
this.originalColumnLabels = this.existingColumns.map(col => col.rawLabel ?? col.label)
@@ -385,13 +385,13 @@ class TpenManageColumns extends HTMLElement {
385385
lines: column.lines ?? []
386386
})) || []
387387
const assignedAnnotationIds = []
388-
await this.columnLabelCheck()
388+
this.columnLabelCheck()
389389
this.existingColumns.forEach(column => {
390390
column.lines.forEach(annoId => assignedAnnotationIds.push({
391391
lineId: annoId, columnLabel: column.label
392392
}))
393393
})
394-
this.totalIds = annotations.filter(anno => !assignedAnnotationIds.find(a => a.lineId === anno.lineId)).map(a => a.lineId)
394+
this.totalIds = annotations.filter(anno => !assignedAnnotationIds.some(a => a.lineId === anno.lineId)).map(a => a.lineId)
395395
localStorage.setItem('annotationsState', JSON.stringify({
396396
remainingIDs: this.totalIds,
397397
selectedIDs: []
@@ -409,7 +409,7 @@ class TpenManageColumns extends HTMLElement {
409409
*/
410410
async reloadColumns() {
411411
try {
412-
await this.columnLabelCheck()
412+
this.columnLabelCheck()
413413
// Always ensure TPEN.activeProject is source of truth
414414
this.syncProjectColumnsFromExisting()
415415
this.selectedBoxes = []
@@ -439,7 +439,7 @@ class TpenManageColumns extends HTMLElement {
439439
.find(p => p.id.split('/').pop() === this.annotationPageID)
440440
}
441441

442-
generateColumnId(label) {
442+
generateColumnId() {
443443
return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
444444
}
445445

@@ -476,8 +476,6 @@ class TpenManageColumns extends HTMLElement {
476476
label: rawLabel,
477477
lines: [...new Set(column.lines ?? [])]
478478
}
479-
// Ensure id is consistent
480-
if (!updated.id && updated._id) updated.id = updated._id
481479
return updated
482480
})
483481
}
@@ -494,11 +492,11 @@ class TpenManageColumns extends HTMLElement {
494492
}
495493

496494
refreshFromCache() {
497-
if (!this.cachedImageInfo || !this.cachedAnnotations) return false
495+
if (!this.cachedImageInfo || !this.cachedAnnotations?.length) return false
498496
const { imgWidth, imgHeight } = this.cachedImageInfo
499497
const assignedAnnotationIds = this.buildAssignedAnnotations()
500498
this.totalIds = this.cachedAnnotations
501-
.filter(anno => !assignedAnnotationIds.find(a => a.lineId === anno.lineId))
499+
.filter(anno => !assignedAnnotationIds.some(a => a.lineId === anno.lineId))
502500
.map(a => a.lineId)
503501
localStorage.setItem('annotationsState', JSON.stringify({
504502
remainingIDs: this.totalIds,
@@ -511,7 +509,7 @@ class TpenManageColumns extends HTMLElement {
511509
}
512510

513511
mergeColumnsLocal(newLabel, columnIndexes) {
514-
const mergeIndexes = [...new Set(columnIndexes)].filter(i => i !== '').sort((a, b) => b - a)
512+
const mergeIndexes = [...new Set(columnIndexes)].sort((a, b) => b - a)
515513
const mergedLines = []
516514
mergeIndexes.forEach(index => {
517515
const column = this.existingColumns[index]
@@ -673,8 +671,9 @@ class TpenManageColumns extends HTMLElement {
673671
}
674672

675673
const duplicate = this.existingColumns.some(col => {
676-
const existingLabel = (col.label ?? "").toString().trim()
677-
return existingLabel === newLabel
674+
const displayLabel = (col.label ?? "").toString().trim()
675+
const rawLabel = (col.rawLabel ?? "").toString().trim()
676+
return displayLabel === newLabel || rawLabel === newLabel
678677
})
679678
if (duplicate) {
680679
return TPEN.eventDispatcher.dispatch("tpen-toast", {
@@ -698,12 +697,14 @@ class TpenManageColumns extends HTMLElement {
698697
TPEN.eventDispatcher.dispatch("tpen-toast", {
699698
status: "success", message: 'Columns merged successfully.'
700699
})
701-
let data = null
702-
try { data = await res.json() } catch {}
700+
const data = await res.json()
703701
if (!this.updateColumnsFromResponse(data)) {
704702
this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
705703
}
706704
await this.reloadColumns()
705+
// Reset workspace UI to avoid stale state
706+
this.mergeColumnsCheckbox.checked = false
707+
this.handleModeChange()
707708
} catch (error) {
708709
TPEN.eventDispatcher.dispatch("tpen-toast", {
709710
status: "error", message: error.message
@@ -800,12 +801,14 @@ class TpenManageColumns extends HTMLElement {
800801
TPEN.eventDispatcher.dispatch("tpen-toast", {
801802
status: "success", message: 'Column extended successfully.'
802803
})
803-
let data = null
804-
try { data = await res.json() } catch {}
804+
const data = await res.json()
805805
if (!this.updateColumnsFromResponse(data)) {
806-
this.extendColumnLocal(columnToExtend, annotationIdsToAdd)
806+
this.extendColumnLocal(this.originalColumnLabels[columnToExtend], annotationIdsToAdd)
807807
}
808808
await this.reloadColumns()
809+
// Reset workspace UI to avoid stale state
810+
this.extendColumnCheckbox.checked = false
811+
this.handleModeChange()
809812
} catch (error) {
810813
TPEN.eventDispatcher.dispatch("tpen-toast", {
811814
status: "error", message: error.message
@@ -1023,8 +1026,7 @@ class TpenManageColumns extends HTMLElement {
10231026
TPEN.eventDispatcher.dispatch("tpen-toast", {
10241027
status: "success", message: 'Column created successfully.'
10251028
})
1026-
let data = null
1027-
try { data = await res.json() } catch {}
1029+
const data = await res.json()
10281030
if (!this.updateColumnsFromResponse(data)) {
10291031
this.createColumnLocal(columnLabel, selectedIDs)
10301032
}
@@ -1049,8 +1051,7 @@ class TpenManageColumns extends HTMLElement {
10491051
TPEN.eventDispatcher.dispatch("tpen-toast", {
10501052
status: "info", message: 'All columns cleared successfully.'
10511053
})
1052-
let data = null
1053-
try { data = await res.json() } catch {}
1054+
const data = await res.json()
10541055
if (!this.updateColumnsFromResponse(data)) {
10551056
this.clearColumnsLocal()
10561057
}

0 commit comments

Comments
 (0)