From d10b304cf7081a6e1ca96899a710cbf3d75bde09 Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Mon, 11 May 2026 11:07:40 -0500 Subject: [PATCH 1/2] fix annotation list class ordering --- CHANGELOG.md | 4 ++ package.json | 2 +- src/toolbox_items/annotation_list.ts | 59 +++++++++++++++++++++++----- src/version.js | 2 +- tests/e2e/annotation_list.spec.js | 38 ++++++++++++++++++ 5 files changed, 94 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eae513b4..2e5beece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented here. ## [unreleased] +## [0.23.6] - May 11th, 2026 +- Fix `AnnotationList` class ordering to match the `AnnotationID` toolbox item ordering, which is based on the order of annotations in the subtask's annotation array. +- Add local storage of checkbox options for the `AnnotationList` toolbox item + ## [0.23.5] - May 6th, 2026 - Fix bug where on some browsers, middle-click-drag when annotations were vanished would trigger auto-scroll rather than the normal pan behavior. diff --git a/package.json b/package.json index 7af4ef34..9989afa6 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "ulabel", "description": "An image annotation tool.", - "version": "0.23.5", + "version": "0.23.6", "main": "dist/ulabel.min.js", "module": "dist/ulabel.min.js", "types": "index.d.ts", diff --git a/src/toolbox_items/annotation_list.ts b/src/toolbox_items/annotation_list.ts index 50ab7f7e..462fe0a7 100644 --- a/src/toolbox_items/annotation_list.ts +++ b/src/toolbox_items/annotation_list.ts @@ -272,12 +272,14 @@ export class AnnotationListToolboxItem extends ToolboxItem { // Show/hide deprecated annotations checkbox $(document).on("change.ulabel", "#annotation-list-show-deprecated", (e) => { this.show_deprecated = (e.target as HTMLInputElement).checked; + set_local_storage_item("ulabel_annotation_list_show_deprecated", this.show_deprecated ? "true" : "false"); this.update_list(); }); // Group by class checkbox $(document).on("change.ulabel", "#annotation-list-group-by-class", (e) => { this.group_by_class = (e.target as HTMLInputElement).checked; + set_local_storage_item("ulabel_annotation_list_group_by_class", this.group_by_class ? "true" : "false"); this.update_list(); }); @@ -448,11 +450,30 @@ export class AnnotationListToolboxItem extends ToolboxItem { groups[class_id].push(annotation); } + // Build the render order: first walk class_defs in declared order so the + // group headers appear in the same order as the AnnotationIDToolboxItem. + // Then append any orphan class_ids (not in class_defs) as a defensive + // fallback so we never silently drop annotations. + const ordered_class_ids: number[] = []; + const seen_class_ids: Set = new Set(); + for (const def of subtask.class_defs) { + if (groups[def.id]) { + ordered_class_ids.push(def.id); + seen_class_ids.add(def.id); + } + } + const orphan_class_ids = Object.keys(groups) + .map((id_str) => parseInt(id_str)) + .filter((id) => !seen_class_ids.has(id)) + .sort((a, b) => a - b); + for (const id of orphan_class_ids) { + ordered_class_ids.push(id); + } + // Build HTML for each group let html = ""; - for (const class_id_str in groups) { - const class_id = parseInt(class_id_str); + for (const class_id of ordered_class_ids) { const group_annotations = groups[class_id]; const class_def = subtask.class_defs.find((def) => def.id === class_id); const class_name = class_def ? class_def.name : "Unknown"; @@ -604,23 +625,43 @@ export class AnnotationListToolboxItem extends ToolboxItem { * Code called after all of ULabel's constructor and initialization code is called */ public after_init(): void { - // Restore collapsed state from localStorage - this.restore_collapsed_state(); + // Restore persisted UI state from localStorage + this.restore_persisted_state(); // Initial list update this.update_list(); } /** - * Restore the collapsed state from localStorage + * Restore persisted UI state (collapsed flag + checkbox toggles) from localStorage. */ - private restore_collapsed_state(): void { - const stored_state = get_local_storage_item("ulabel_annotation_list_collapsed"); - if (stored_state === "false") { + private restore_persisted_state(): void { + const stored_collapsed = get_local_storage_item("ulabel_annotation_list_collapsed"); + if (stored_collapsed === "false") { this.is_collapsed = false; - } else if (stored_state === "true") { + } else if (stored_collapsed === "true") { this.is_collapsed = true; } + + const stored_show_deprecated = get_local_storage_item("ulabel_annotation_list_show_deprecated"); + if (stored_show_deprecated === "true") { + this.show_deprecated = true; + } else if (stored_show_deprecated === "false") { + this.show_deprecated = false; + } + + const stored_group_by_class = get_local_storage_item("ulabel_annotation_list_group_by_class"); + if (stored_group_by_class === "true") { + this.group_by_class = true; + } else if (stored_group_by_class === "false") { + this.group_by_class = false; + } + + // Reflect restored values into the checkbox DOM so the UI matches state. + const show_deprecated_input = document.querySelector("#annotation-list-show-deprecated"); + if (show_deprecated_input) show_deprecated_input.checked = this.show_deprecated; + const group_by_class_input = document.querySelector("#annotation-list-group-by-class"); + if (group_by_class_input) group_by_class_input.checked = this.group_by_class; } /** diff --git a/src/version.js b/src/version.js index ea7e4054..7b86168c 100644 --- a/src/version.js +++ b/src/version.js @@ -1 +1 @@ -export const ULABEL_VERSION = "0.23.5"; +export const ULABEL_VERSION = "0.23.6"; diff --git a/tests/e2e/annotation_list.spec.js b/tests/e2e/annotation_list.spec.js index 1ff5e0d4..8f47b03d 100644 --- a/tests/e2e/annotation_list.spec.js +++ b/tests/e2e/annotation_list.spec.js @@ -121,4 +121,42 @@ test.describe("Annotation List Grouping", () => { expect(await header.count()).toBeGreaterThan(0); } }); + + test("class group headers appear in class_defs order, not numeric class id order", async ({ page }) => { + await wait_for_ulabel_init(page); + + // multi-class.html defines class_defs as [Sedan(10), SUV(11), Truck(12)]. + // Draw two annotations: one with the default class (Sedan), one after + // switching to the third class (Truck). SUV is intentionally left empty. + await draw_bbox(page, [100, 100], [200, 200]); + await page.mouse.move(300, 300); + await page.keyboard.press("3"); + await draw_bbox(page, [300, 300], [400, 400]); + + // Enable group-by-class and confirm the natural ordering matches class_defs + // (Sedan before Truck). + await toggle_group_by_class(page, true); + let header_texts = await page.locator(".annotation-list-class-group-header").allTextContents(); + // headers contain the class name plus a count like "(1)"; strip whitespace + let names_in_order = header_texts.map((t) => t.trim().split(/\s+/)[0]); + expect(names_in_order).toEqual(["Sedan", "Truck"]); + + // Now reverse class_defs at runtime. With the old (buggy) implementation + // the headers would still appear in numeric class_id order + // ([Sedan(10), Truck(12)]). With the fix they should match the new + // class_defs order ([Truck, Sedan]). + await page.evaluate(() => { + const subtask = window.ulabel.subtasks[window.ulabel.state.current_subtask]; + subtask.class_defs.reverse(); + }); + + // Force the list to re-render. Toggling the checkbox off + on triggers + // build_grouped_list_html. + await toggle_group_by_class(page, false); + await toggle_group_by_class(page, true); + + header_texts = await page.locator(".annotation-list-class-group-header").allTextContents(); + names_in_order = header_texts.map((t) => t.trim().split(/\s+/)[0]); + expect(names_in_order).toEqual(["Truck", "Sedan"]); + }); }); From cf70f64c17e71207d1dd463b3cde2e436abe861f Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Mon, 11 May 2026 11:44:54 -0500 Subject: [PATCH 2/2] Fix test fail due to missing keybind, update changelog wording and add helper function to build class def map --- CHANGELOG.md | 2 +- src/toolbox_items/annotation_list.ts | 21 +++++++++++++++++++-- tests/e2e/annotation_list.spec.js | 22 ++++++++++++---------- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e5beece..37f24804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented here. ## [unreleased] ## [0.23.6] - May 11th, 2026 -- Fix `AnnotationList` class ordering to match the `AnnotationID` toolbox item ordering, which is based on the order of annotations in the subtask's annotation array. +- Fix `AnnotationList` class ordering to match the `AnnotationID` toolbox item ordering, which is based on the configured class definition order in the subtask's `classes` array - Add local storage of checkbox options for the `AnnotationList` toolbox item ## [0.23.5] - May 6th, 2026 diff --git a/src/toolbox_items/annotation_list.ts b/src/toolbox_items/annotation_list.ts index 462fe0a7..0054e9d4 100644 --- a/src/toolbox_items/annotation_list.ts +++ b/src/toolbox_items/annotation_list.ts @@ -409,12 +409,13 @@ export class AnnotationListToolboxItem extends ToolboxItem { * Build HTML for flat (non-grouped) list */ private build_flat_list_html(annotations: ULabelAnnotation[], subtask: ULabelSubtask): string { + const class_def_by_id = this.build_class_def_by_id(subtask); let html = ""; for (let i = 0; i < annotations.length; i++) { const annotation = annotations[i]; const class_id = this.get_annotation_class_id(annotation); - const class_def = subtask.class_defs.find((def) => def.id === class_id); + const class_def = class_def_by_id.get(class_id); const class_name = class_def ? class_def.name : "Unknown"; const color = this.ulabel.color_info[class_id] || "#cccccc"; const svg = this.get_spatial_type_svg(annotation.spatial_type!, color); @@ -439,6 +440,8 @@ export class AnnotationListToolboxItem extends ToolboxItem { * Build HTML for grouped (by class) list */ private build_grouped_list_html(annotations: ULabelAnnotation[], subtask: ULabelSubtask): string { + const class_def_by_id = this.build_class_def_by_id(subtask); + // Group annotations by class const groups: { [class_id: number]: ULabelAnnotation[] } = {}; @@ -475,7 +478,7 @@ export class AnnotationListToolboxItem extends ToolboxItem { for (const class_id of ordered_class_ids) { const group_annotations = groups[class_id]; - const class_def = subtask.class_defs.find((def) => def.id === class_id); + const class_def = class_def_by_id.get(class_id); const class_name = class_def ? class_def.name : "Unknown"; const color = this.ulabel.color_info[class_id] || "#cccccc"; @@ -584,6 +587,20 @@ export class AnnotationListToolboxItem extends ToolboxItem { return class_id; } + /** + * Build a Map from class id to ClassDefinition for the given subtask. The + * returned map is intended to be used for the duration of a single render so + * that per-annotation class lookups don't repeat a linear search through + * `subtask.class_defs`. + */ + private build_class_def_by_id(subtask: ULabelSubtask): Map { + const map = new Map(); + for (const def of subtask.class_defs) { + map.set(def.id, def); + } + return map; + } + /** * Get the HTML for this toolbox item */ diff --git a/tests/e2e/annotation_list.spec.js b/tests/e2e/annotation_list.spec.js index 8f47b03d..e30a0d13 100644 --- a/tests/e2e/annotation_list.spec.js +++ b/tests/e2e/annotation_list.spec.js @@ -126,25 +126,27 @@ test.describe("Annotation List Grouping", () => { await wait_for_ulabel_init(page); // multi-class.html defines class_defs as [Sedan(10), SUV(11), Truck(12)]. - // Draw two annotations: one with the default class (Sedan), one after - // switching to the third class (Truck). SUV is intentionally left empty. + // Use Sedan + SUV here because those are the two classes with default + // keybinds ("1" and "2") in the demo. Truck (id 12) has no default + // keybind, so pressing "3" would be a no-op and silently leave the + // active class unchanged. await draw_bbox(page, [100, 100], [200, 200]); await page.mouse.move(300, 300); - await page.keyboard.press("3"); + await page.keyboard.press("2"); await draw_bbox(page, [300, 300], [400, 400]); - // Enable group-by-class and confirm the natural ordering matches class_defs - // (Sedan before Truck). + // Enable group-by-class and confirm the natural ordering matches + // class_defs (Sedan before SUV). await toggle_group_by_class(page, true); let header_texts = await page.locator(".annotation-list-class-group-header").allTextContents(); // headers contain the class name plus a count like "(1)"; strip whitespace let names_in_order = header_texts.map((t) => t.trim().split(/\s+/)[0]); - expect(names_in_order).toEqual(["Sedan", "Truck"]); + expect(names_in_order).toEqual(["Sedan", "SUV"]); // Now reverse class_defs at runtime. With the old (buggy) implementation - // the headers would still appear in numeric class_id order - // ([Sedan(10), Truck(12)]). With the fix they should match the new - // class_defs order ([Truck, Sedan]). + // the headers would still appear in ascending numeric class_id order + // ([Sedan(10), SUV(11)]). With the fix they should follow the new + // class_defs order ([SUV, Sedan]). await page.evaluate(() => { const subtask = window.ulabel.subtasks[window.ulabel.state.current_subtask]; subtask.class_defs.reverse(); @@ -157,6 +159,6 @@ test.describe("Annotation List Grouping", () => { header_texts = await page.locator(".annotation-list-class-group-header").allTextContents(); names_in_order = header_texts.map((t) => t.trim().split(/\s+/)[0]); - expect(names_in_order).toEqual(["Truck", "Sedan"]); + expect(names_in_order).toEqual(["SUV", "Sedan"]); }); });