Skip to content

Commit 3107dfd

Browse files
revert breaking changes
1 parent bb688d9 commit 3107dfd

9 files changed

Lines changed: 210 additions & 23 deletions

File tree

index.d.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export type ClassDefinition = {
6565
name: string;
6666
id: number;
6767
color: string;
68-
keybind?: string;
68+
keybind: string | null;
6969
};
7070

7171
export type SliderInfo = {
@@ -135,7 +135,7 @@ export type ULabelAnnotations = { [key: string]: ULabelAnnotation[] };
135135

136136
export type ULabelSubmitData = {
137137
annotations: ULabelAnnotations;
138-
task_meta: object;
138+
task_meta: object | null;
139139
};
140140
export type ULabelSubmitHandler = (submitData: ULabelSubmitData) => void;
141141

@@ -277,7 +277,7 @@ export class ULabel {
277277
is_editing_keybind: boolean;
278278
// Original keybind storage
279279
original_config_keybinds?: { [config_key: string]: string };
280-
original_class_keybinds?: { [class_id: number]: string };
280+
original_class_keybinds?: { [class_id: number]: string | null };
281281
// Render state
282282
// TODO (joshua-dean): this is never assigned, is it used?
283283
demo_canvas_context: CanvasRenderingContext2D;
@@ -351,7 +351,7 @@ export class ULabel {
351351
public show_annotation_mode(
352352
target_jq?: JQuery<HTMLElement>, // TODO (joshua-dean): validate this type
353353
): void;
354-
public update_frame(delta?: number, new_frame?: number): void;
354+
public update_frame(delta?: number | null, new_frame?: number | null): void;
355355
public rebuild_containing_box(actid: string, ignore_final?: boolean, subtask?: string): void;
356356
public update_filter_distance_during_polyline_move(
357357
annotation_id: string,
@@ -368,7 +368,7 @@ export class ULabel {
368368
public get_keypoint_slider_value(): number | null;
369369
public get_distance_filter_value(): DistanceFromPolylineClasses | null;
370370
public fly_to_next_annotation(increment: number, max_zoom?: number): boolean;
371-
public fly_to_annotation_id(annotation_id: string, subtask_key?: string, max_zoom?: number): boolean;
371+
public fly_to_annotation_id(annotation_id: string, subtask_key?: string | null, max_zoom?: number): boolean;
372372
public fly_to_annotation(annotation: ULabelAnnotation, subtask_key?: string, max_zoom?: number): boolean;
373373

374374
// Brush
@@ -403,8 +403,8 @@ export class ULabel {
403403

404404
// Annotation lifecycle
405405
// TODO (joshua-dean): type for redo_payload
406-
public begin_annotation(mouse_event: JQuery.TriggeredEvent | undefined, annotation_id?: string, redo_payload?: object): void;
407-
public continue_annotation(mouse_event: JQuery.TriggeredEvent | undefined, is_click?: boolean, annotation_id?: string, redo_payload?: object): void;
406+
public begin_annotation(mouse_event: JQuery.TriggeredEvent | null | undefined, annotation_id?: string | null, redo_payload?: object | null): void;
407+
public continue_annotation(mouse_event: JQuery.TriggeredEvent | null | undefined, is_click?: boolean, annotation_id?: string | null, redo_payload?: object | null): void;
408408
public delete_annotation(
409409
annotation_id: string,
410410
redoing?: boolean,
@@ -496,14 +496,14 @@ export class ULabel {
496496

497497
// Edit suggestions
498498
public suggest_edits(
499-
mouse_event?: JQuery.TriggeredEvent,
500-
nonspatial_id?: string,
499+
mouse_event?: JQuery.TriggeredEvent | null,
500+
nonspatial_id?: string | null,
501501
force_refresh?: boolean,
502502
): void;
503503
public show_global_edit_suggestion(
504504
annid: string,
505-
offset?: Offset,
506-
nonspatial_id?: string,
505+
offset?: Offset | null,
506+
nonspatial_id?: string | null,
507507
): void;
508508
public hide_global_edit_suggestion(): void;
509509
public hide_edit_suggestion(): void;

src/actions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ function on_finish_annotation_spatial_modification(
367367
ulabel.rebuild_containing_box(action.annotation_id!);
368368
ulabel.redraw_annotation(action.annotation_id!);
369369
// Update dialogs
370-
ulabel.suggest_edits(undefined, undefined, true);
370+
ulabel.suggest_edits(null, null, true);
371371
// Update the toolbox
372372
ulabel.update_filter_distance(action.annotation_id!);
373373
ulabel.toolbox.redraw_update_items(ulabel);
@@ -409,7 +409,7 @@ function on_annotation_deletion(
409409
// Ensure there are no lingering enders
410410
ulabel.destroy_polygon_ender(action.annotation_id!);
411411
// Update dialogs
412-
ulabel.suggest_edits(undefined, undefined, true);
412+
ulabel.suggest_edits(null, null, true);
413413
// Update the toolbox
414414
ulabel.toolbox.redraw_update_items(ulabel);
415415
}
@@ -436,7 +436,7 @@ function on_annotation_id_change(
436436
// Hide the large ID dialog after the user has made a selection
437437
ulabel.hide_id_dialog();
438438
}
439-
ulabel.suggest_edits(undefined, undefined, true);
439+
ulabel.suggest_edits(null, null, true);
440440

441441
// Determine if we need to update the filter distance
442442
// If the filter_distance_toolbox_item exists,

src/initializer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ function store_original_keybinds(ulabel: ULabel) {
7777
ulabel.state["original_config_keybinds"] = original_config_keybinds;
7878

7979
// Store original class keybinds in the ULabel state for later reference
80-
const original_class_keybinds: { [class_id: number]: string } = {};
80+
const original_class_keybinds: { [class_id: number]: string | null } = {};
8181
for (const subtask_key in ulabel.subtasks) {
8282
const subtask = ulabel.subtasks[subtask_key];
8383
if (subtask.class_defs) {
8484
for (const class_def of subtask.class_defs) {
85-
original_class_keybinds[class_def.id] = class_def.keybind || "";
85+
original_class_keybinds[class_def.id] = class_def.keybind;
8686
}
8787
}
8888
}

src/listeners.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ export function create_ulabel_listeners(
577577
(mouse_event) => {
578578
// Show thumbnail for idd
579579
ulabel.suggest_edits(
580-
undefined,
580+
null,
581581
$(mouse_event.currentTarget).attr("id")!.substring("row__".length),
582582
);
583583
},
@@ -594,7 +594,7 @@ export function create_ulabel_listeners(
594594
) {
595595
return;
596596
}
597-
ulabel.suggest_edits(undefined);
597+
ulabel.suggest_edits(null);
598598
},
599599
);
600600

src/toolbox_items/annotation_list.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export class AnnotationListToolboxItem extends ToolboxItem {
285285
$(document).on("click.ulabel", ".annotation-list-item", (e) => {
286286
const annotation_id = $(e.currentTarget).data("annotation-id");
287287
if (annotation_id) {
288-
this.ulabel.fly_to_annotation_id(annotation_id, undefined, this.ulabel.config.fly_to_max_zoom);
288+
this.ulabel.fly_to_annotation_id(annotation_id, null, this.ulabel.config.fly_to_max_zoom);
289289
}
290290
});
291291

@@ -298,7 +298,7 @@ export class AnnotationListToolboxItem extends ToolboxItem {
298298
$(e.currentTarget).addClass("highlighted");
299299

300300
// Show the global edit suggestion (ID dialog)
301-
this.ulabel.show_global_edit_suggestion(annotation_id, undefined, undefined);
301+
this.ulabel.show_global_edit_suggestion(annotation_id, null, null);
302302

303303
// Set edit_candidate to allow delete keybind to work
304304
const current_subtask = this.ulabel.get_current_subtask();

src/toolbox_items/keybinds.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ export class KeybindsToolboxItem extends ToolboxItem {
602602
/**
603603
* Get original class keybinds (before customization)
604604
*/
605-
private get_original_class_keybinds(): { [class_id: number]: string } {
605+
private get_original_class_keybinds(): { [class_id: number]: string | null } {
606606
// Get from ULabel state (stored during initialization)
607607
return this.ulabel.state["original_class_keybinds"] || {};
608608
}

src/toolbox_items/submit_buttons.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ export class SubmitButtons extends ToolboxItem {
5959
button.appendChild(animation);
6060

6161
// Create the submit payload
62-
const submit_payload: { task_meta: object; annotations: Record<string, ULabelAnnotation[]> } = {
63-
task_meta: ulabel.config["task_meta"] || {},
62+
const submit_payload: { task_meta: object | null; annotations: Record<string, ULabelAnnotation[]> } = {
63+
task_meta: ulabel.config["task_meta"],
6464
annotations: {},
6565
};
6666

tests/e2e/api-behavior.spec.js

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* E2E tests for ULabel API behavior contracts.
3+
*
4+
* Verifies that null-dependent control flow in public methods like
5+
* suggest_edits, fly_to_annotation_id, and show_global_edit_suggestion
6+
* behaves correctly.
7+
*/
8+
import { test, expect } from "./fixtures";
9+
import { draw_bbox, draw_point } from "../testing-utils/drawing_utils";
10+
import { wait_for_ulabel_init } from "../testing-utils/init_utils";
11+
12+
test.describe("ULabel API Behavior", () => {
13+
test("suggest_edits with no arguments should not throw", async ({ page }) => {
14+
await wait_for_ulabel_init(page);
15+
16+
// Create an annotation so there's something to suggest edits for
17+
await draw_bbox(page, [100, 100], [200, 200]);
18+
19+
// suggest_edits() with no arguments should use null defaults internally
20+
// and fall through to last_move. Should not throw.
21+
const result = await page.evaluate(() => {
22+
try {
23+
window.ulabel.suggest_edits();
24+
return { success: true };
25+
} catch (e) {
26+
return { success: false, error: e.message };
27+
}
28+
});
29+
30+
expect(result.success).toBe(true);
31+
});
32+
33+
test("suggest_edits with null nonspatial_id should not create bad candidate", async ({ page }) => {
34+
await wait_for_ulabel_init(page);
35+
36+
await draw_bbox(page, [100, 100], [200, 200]);
37+
38+
// Passing null for nonspatial_id should NOT create a best_candidate
39+
// (the check is: if (nonspatial_id !== null))
40+
const result = await page.evaluate(() => {
41+
try {
42+
window.ulabel.suggest_edits(null, null, true);
43+
return { success: true };
44+
} catch (e) {
45+
return { success: false, error: e.message };
46+
}
47+
});
48+
49+
expect(result.success).toBe(true);
50+
});
51+
52+
test("fly_to_annotation_id with null subtask_key should not switch subtasks", async ({ page }) => {
53+
await wait_for_ulabel_init(page);
54+
55+
// Create an annotation to fly to
56+
await draw_point(page, [150, 150]);
57+
58+
const result = await page.evaluate(() => {
59+
const annotations = window.ulabel.get_current_subtask().annotations;
60+
const annotation_id = annotations.ordering[0];
61+
const initial_subtask = window.ulabel.state.current_subtask;
62+
63+
// null subtask_key should NOT trigger set_subtask
64+
window.ulabel.fly_to_annotation_id(annotation_id, null);
65+
66+
return {
67+
subtask_unchanged: window.ulabel.state.current_subtask === initial_subtask,
68+
};
69+
});
70+
71+
expect(result.subtask_unchanged).toBe(true);
72+
});
73+
74+
test("show_global_edit_suggestion with null nonspatial_id uses spatial path", async ({ page }) => {
75+
await wait_for_ulabel_init(page);
76+
77+
// Create a spatial annotation
78+
await draw_bbox(page, [100, 100], [200, 200]);
79+
80+
// With null nonspatial_id, should use the spatial path (containing_box)
81+
// not the nonspatial path (reclf__ DOM element)
82+
const result = await page.evaluate(() => {
83+
const annotations = window.ulabel.get_current_subtask().annotations;
84+
const annotation_id = annotations.ordering[0];
85+
86+
try {
87+
window.ulabel.show_global_edit_suggestion(annotation_id, null, null);
88+
return { success: true };
89+
} catch (e) {
90+
return { success: false, error: e.message };
91+
}
92+
});
93+
94+
expect(result.success).toBe(true);
95+
});
96+
97+
test("submit payload preserves task_meta value from config", async ({ page }) => {
98+
await wait_for_ulabel_init(page);
99+
100+
// Check that task_meta in config is preserved as-is
101+
const task_meta = await page.evaluate(() => {
102+
return window.ulabel.config.task_meta;
103+
});
104+
105+
// task_meta should be whatever the demo page configured (likely {} or an object)
106+
// The key assertion: it should NOT be unexpectedly converted
107+
expect(task_meta).not.toBeUndefined();
108+
});
109+
110+
test("class keybinds should be null when not configured", async ({ page }) => {
111+
await wait_for_ulabel_init(page);
112+
113+
const keybind_values = await page.evaluate(() => {
114+
const subtask = window.ulabel.get_current_subtask();
115+
return subtask.class_defs.map((cd) => ({
116+
id: cd.id,
117+
keybind: cd.keybind,
118+
keybind_is_null: cd.keybind === null,
119+
keybind_is_undefined: cd.keybind === undefined,
120+
}));
121+
});
122+
123+
// Classes without configured keybinds should have null (not undefined or "")
124+
for (const entry of keybind_values) {
125+
if (entry.keybind === null) {
126+
expect(entry.keybind_is_null).toBe(true);
127+
expect(entry.keybind_is_undefined).toBe(false);
128+
}
129+
}
130+
});
131+
});

tests/ulabel.test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,60 @@ describe("ULabel Core Functionality", () => {
142142
expect(typeof new_id).toBe("number");
143143
});
144144
});
145+
146+
describe("Class Keybind Storage", () => {
147+
test("class_def.keybind should be null when not provided", () => {
148+
const ulabel = new ULabel(mock_config);
149+
const class_defs = ulabel.subtasks.test_task.class_defs;
150+
151+
// Class without keybind should have null, not undefined or ""
152+
expect(class_defs[0].keybind).toBe(null);
153+
});
154+
155+
test("class_def.keybind should preserve provided value", () => {
156+
const config = {
157+
...mock_config,
158+
subtasks: {
159+
test_task: {
160+
display_name: "Test Task",
161+
classes: [
162+
{ name: "Class1", id: 1, color: "red", keybind: "1" },
163+
{ name: "Class2", id: 2, color: "blue", keybind: "2" },
164+
],
165+
allowed_modes: ["bbox", "polygon", "point"],
166+
},
167+
},
168+
};
169+
const ulabel = new ULabel(config);
170+
const class_defs = ulabel.subtasks.test_task.class_defs;
171+
172+
expect(class_defs[0].keybind).toBe("1");
173+
expect(class_defs[1].keybind).toBe("2");
174+
});
175+
});
176+
177+
describe("Configuration Defaults", () => {
178+
test("task_meta should default to empty object when not configured", () => {
179+
const ulabel = new ULabel(mock_config);
180+
expect(ulabel.config.task_meta).toEqual({});
181+
});
182+
183+
test("task_meta should preserve configured value", () => {
184+
const config_with_meta = {
185+
...mock_config,
186+
task_meta: { project: "test" },
187+
};
188+
const ulabel = new ULabel(config_with_meta);
189+
expect(ulabel.config.task_meta).toEqual({ project: "test" });
190+
});
191+
192+
test("task_meta null should be preserved when explicitly set", () => {
193+
const config_with_null_meta = {
194+
...mock_config,
195+
task_meta: null,
196+
};
197+
const ulabel = new ULabel(config_with_null_meta);
198+
expect(ulabel.config.task_meta).toBeNull();
199+
});
200+
});
145201
});

0 commit comments

Comments
 (0)