Skip to content

Commit 188fe1c

Browse files
authored
74 improve code quality of inspector and inspectorComponents (#98)
- Renamed SingleFieldInspectorComponent -> InputFieldInspectorComponent - Combine and simplify event listeners to render inspector - Refactor and de-duplicate inspector components
1 parent cfe8150 commit 188fe1c

5 files changed

Lines changed: 92 additions & 106 deletions

File tree

canvas_editor/static/js/editor/inspector.mjs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import { Picker } from "picker";
2-
import { ItemDeletedEvent } from "deleteCommands";
3-
import { ItemUpdatedEvent } from "updateCommands";
42
import { CanvasObject } from "canvasObject";
53

64
/**
@@ -24,18 +22,11 @@ export class Inspector {
2422
this.#inspectorElem = document.getElementById("inspector");
2523

2624
const canvas = document.getElementById("canvas");
27-
canvas.addEventListener("itemSelected", () => {
28-
this.#render();
29-
});
3025

31-
canvas.addEventListener("itemDeleted", (/** @type {ItemDeletedEvent}*/ event) => {
32-
if (this.#objectList.length == 1 && event.detail.item == this.#objectList[0]) {
33-
this.#render();
34-
}
35-
});
26+
const rerenderEvents = ["itemSelected", "itemUpdated", "itemDeleted"];
3627

37-
canvas.addEventListener("itemUpdated", (/** @type {ItemUpdatedEvent} */ event) => {
38-
if (this.#objectList.length == 1 && event.detail.item == this.#objectList[0]) this.#render();
28+
rerenderEvents.forEach((eventName) => {
29+
canvas.addEventListener(eventName, () => this.#render());
3930
});
4031

4132
this.#render();

canvas_editor/static/js/editor/inspectorComponents.mjs

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { abstractClassError, methodMustBeImplementedError } from "message_dict";
55
* Represents a single component of the inspector
66
*/
77
export class InspectorComponent {
8+
#hasBorder = false;
89
/**
910
* Creates a new inspector component.
1011
* @throws {Error} if trying to instantiate this abstract class directly
@@ -25,22 +26,63 @@ export class InspectorComponent {
2526

2627
/**
2728
* Disables the border around the component
28-
* @throws {Error} if this method is not implemented in a subclass
2929
*/
3030
disableBorder() {
31-
throw new Error(methodMustBeImplementedError);
31+
this.#hasBorder = false;
32+
}
33+
34+
/**
35+
* Enables the border around the component
36+
*/
37+
enableBorder() {
38+
this.#hasBorder = true;
39+
}
40+
41+
/**
42+
* Gets whether the component has a border or not
43+
* @returns {boolean} whether the component has a border or not
44+
*/
45+
get hasBorder() {
46+
return this.#hasBorder;
47+
}
48+
49+
/**
50+
* Creates the wrapper element for the component
51+
* @param {string[]} classes the classes to add to the wrapper
52+
* @returns {HTMLElement} the created wrapper element
53+
*/
54+
createWrapper(classes) {
55+
const wrapper = document.createElement("div");
56+
wrapper.classList.add(...classes);
57+
if (this.hasBorder) {
58+
wrapper.classList.add("border");
59+
}
60+
return wrapper;
61+
}
62+
63+
/**
64+
* Creates the field name element
65+
* @param {HTMLElement} wrapper the wrapper to add the field name to
66+
* @param {string} fieldNametext the text of the field name
67+
* @returns {HTMLElement} the created field name element
68+
*/
69+
createFieldNameElement(wrapper, fieldNametext) {
70+
const fieldName = document.createElement("div");
71+
fieldName.classList.add("d-flex", "align-items-center", "text-nowrap");
72+
fieldName.innerText = fieldNametext + ":";
73+
wrapper.appendChild(fieldName);
74+
return fieldName;
3275
}
3376
}
3477

3578
/**
3679
* A single field component consists out of a name and the corresponding input field
3780
*/
38-
export class SingleFieldInspectorComponent extends InspectorComponent {
81+
export class InputFieldInspectorComponent extends InspectorComponent {
3982
#fieldName;
4083
#fieldType;
4184
#getFieldValueFunc;
4285
#saveFunc;
43-
#hasBorder;
4486
#InputLimitBottom;
4587

4688
/**
@@ -57,25 +99,18 @@ export class SingleFieldInspectorComponent extends InspectorComponent {
5799
this.#fieldType = fieldType;
58100
this.#getFieldValueFunc = getFieldValueFunc;
59101
this.#saveFunc = saveFunc;
60-
this.#hasBorder = true;
61102
this.#InputLimitBottom = InputLimitBottom;
103+
super.enableBorder();
62104
}
63105

64106
/**
65107
* Renders the component and also adds the necessary logic to updating and saving.
66108
* @returns {HTMLElement} the rendered component
67109
*/
68110
render() {
69-
const wrapper = document.createElement("div");
70-
wrapper.classList.add("d-flex", "p-2", "bg-body", "rounded-2", "gap-2");
71-
if (this.#hasBorder) {
72-
wrapper.classList.add("border");
73-
}
111+
const wrapper = super.createWrapper(["d-flex", "p-2", "bg-body", "rounded-2", "gap-2"]);
74112

75-
const fieldName = document.createElement("div");
76-
fieldName.classList.add("d-flex", "align-items-center", "text-nowrap");
77-
fieldName.innerText = this.#fieldName + ":";
78-
wrapper.appendChild(fieldName);
113+
super.createFieldNameElement(wrapper, this.#fieldName);
79114

80115
const input = document.createElement("input");
81116
input.classList.add("form-control", "rounded-1");
@@ -107,13 +142,6 @@ export class SingleFieldInspectorComponent extends InspectorComponent {
107142

108143
return wrapper;
109144
}
110-
111-
/**
112-
* Disables the border around the component
113-
*/
114-
disableBorder() {
115-
this.#hasBorder = false;
116-
}
117145
}
118146

119147
/**
@@ -140,8 +168,7 @@ export class MultiFieldInspectorComponent extends InspectorComponent {
140168
* @returns {HTMLElement} the rendered component
141169
*/
142170
render() {
143-
const wrapper = document.createElement("div");
144-
wrapper.classList.add("accordion");
171+
const wrapper = super.createWrapper(["accordion"]);
145172
wrapper.id = this.#title;
146173

147174
const accordionItem = document.createElement("div");
@@ -181,7 +208,9 @@ export class MultiFieldInspectorComponent extends InspectorComponent {
181208
bodyWrapper.appendChild(body);
182209

183210
this.#componentList.forEach((component) => {
184-
component.disableBorder();
211+
if (component.hasBorder === true) {
212+
component.disableBorder();
213+
}
185214
body.appendChild(component.render());
186215
});
187216

@@ -197,7 +226,6 @@ export class SelectFieldInspectorComponent extends InspectorComponent {
197226
#options;
198227
#getFieldValueFunc;
199228
#saveFunc;
200-
#hasBorder;
201229

202230
/**
203231
* Creates a new single field component
@@ -212,24 +240,17 @@ export class SelectFieldInspectorComponent extends InspectorComponent {
212240
this.#options = options;
213241
this.#getFieldValueFunc = getFieldValueFunc;
214242
this.#saveFunc = saveFunc;
215-
this.#hasBorder = true;
243+
super.enableBorder();
216244
}
217245

218246
/**
219247
* Renders the component and also adds the necessary logic to updating and saving.
220248
* @returns {HTMLElement} the rendered component
221249
*/
222250
render() {
223-
const wrapper = document.createElement("div");
224-
wrapper.classList.add("d-flex", "p-2", "bg-body", "rounded-3", "gap-2");
225-
if (this.#hasBorder) {
226-
wrapper.classList.add("border");
227-
}
251+
const wrapper = super.createWrapper(["d-flex", "p-2", "bg-body", "rounded-3", "gap-2"]);
228252

229-
const fieldName = document.createElement("div");
230-
fieldName.classList.add("d-flex", "align-items-center", "text-nowrap");
231-
fieldName.innerText = this.#fieldName + ":";
232-
wrapper.appendChild(fieldName);
253+
super.createFieldNameElement(wrapper, this.#fieldName);
233254

234255
const select = document.createElement("select");
235256
select.classList.add("form-select", "rounded-1");
@@ -254,13 +275,6 @@ export class SelectFieldInspectorComponent extends InspectorComponent {
254275

255276
return wrapper;
256277
}
257-
258-
/**
259-
* Disables the border around the component
260-
*/
261-
disableBorder() {
262-
this.#hasBorder = false;
263-
}
264278
}
265279

266280
/**
@@ -273,7 +287,6 @@ export class SliderFieldInspectorComponent extends InspectorComponent {
273287
#max;
274288
#getFieldValueFunc;
275289
#saveFunc;
276-
#hasBorder;
277290

278291
/**
279292
* Creates a new slider component
@@ -292,28 +305,21 @@ export class SliderFieldInspectorComponent extends InspectorComponent {
292305
this.#step = step;
293306
this.#getFieldValueFunc = getFieldValueFunc;
294307
this.#saveFunc = saveFunc;
295-
this.#hasBorder = true;
308+
super.enableBorder();
296309
}
297310

298311
/**
299312
* Renders the component and also adds the necessary logic to updating and saving.
300313
* @returns {HTMLElement} the rendered component
301314
*/
302315
render() {
303-
const wrapper = document.createElement("div");
304-
wrapper.classList.add("d-flex", "flex-column", "p-2", "bg-body", "rounded-3", "gap-2");
305-
if (this.#hasBorder) {
306-
wrapper.classList.add("border");
307-
}
316+
const wrapper = super.createWrapper(["d-flex", "flex-column", "p-2", "bg-body", "rounded-3", "gap-2"]);
308317

309318
const header = document.createElement("div");
310319
header.classList.add("d-flex", "gap-2");
311320
wrapper.appendChild(header);
312321

313-
const fieldName = document.createElement("div");
314-
fieldName.classList.add("d-flex", "align-items-center", "text-nowrap");
315-
fieldName.innerText = this.#fieldName + ":";
316-
header.appendChild(fieldName);
322+
super.createFieldNameElement(header, this.#fieldName);
317323

318324
const input = document.createElement("input");
319325
input.classList.add("form-control", "rounded-1");
@@ -363,13 +369,6 @@ export class SliderFieldInspectorComponent extends InspectorComponent {
363369

364370
return wrapper;
365371
}
366-
367-
/**
368-
* Disables the border around the component
369-
*/
370-
disableBorder() {
371-
this.#hasBorder = false;
372-
}
373372
}
374373

375374
/**
@@ -398,8 +397,7 @@ export class HeaderInspectorComponent extends InspectorComponent {
398397
* @returns {HTMLElement} the rendered component
399398
*/
400399
render() {
401-
const wrapper = document.createElement("div");
402-
wrapper.classList.add("d-flex", "px-1", "gap-1");
400+
const wrapper = super.createWrapper(["d-flex", "px-1", "gap-1"]);
403401

404402
const title = document.createElement("div");
405403
title.classList.add("fw-bolder", "fs-4");
@@ -430,23 +428,20 @@ export class HeaderInspectorComponent extends InspectorComponent {
430428
inputField.type = "text";
431429
inputField.classList.add("form-control", "rounded-1");
432430

433-
// Event listeners for clicking the title
434-
title.addEventListener("click", () => {
431+
/**
432+
* Enables editing of the title field
433+
*/
434+
const enableEditing = () => {
435435
title.innerText = "";
436436
inputField.value = this.#getFieldValueFunc();
437437
title.appendChild(inputField);
438438
inputField.focus();
439439
inputField.select();
440-
});
440+
};
441441

442-
// Event listeners for clicking the edit button
443-
editButton.addEventListener("click", () => {
444-
title.innerText = "";
445-
inputField.value = this.#getFieldValueFunc();
446-
title.appendChild(inputField);
447-
inputField.focus();
448-
inputField.select();
449-
});
442+
// Use the same handler for both events
443+
title.addEventListener("click", enableEditing);
444+
editButton.addEventListener("click", enableEditing);
450445

451446
// duplicate button
452447
const duplicateButton = document.createElement("button");

canvas_editor/static/js/editor/objects/heliostat.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { DeleteHeliostatCommand } from "deleteCommands";
33
import { DuplicateHeliostatCommand } from "duplicateCommands";
44
import {
55
HeaderInspectorComponent,
6-
SingleFieldInspectorComponent,
6+
InputFieldInspectorComponent,
77
MultiFieldInspectorComponent,
88
InspectorComponent,
99
} from "inspectorComponents";
@@ -51,7 +51,7 @@ export class Heliostat extends CanvasObject {
5151
this,
5252
);
5353

54-
const nCoordinate = new SingleFieldInspectorComponent(
54+
const nCoordinate = new InputFieldInspectorComponent(
5555
"N",
5656
"number",
5757
() => this.position.x,
@@ -63,7 +63,7 @@ export class Heliostat extends CanvasObject {
6363
-Infinity,
6464
);
6565

66-
const uCoordinate = new SingleFieldInspectorComponent(
66+
const uCoordinate = new InputFieldInspectorComponent(
6767
"U",
6868
"number",
6969
() => this.position.y,
@@ -75,7 +75,7 @@ export class Heliostat extends CanvasObject {
7575
0,
7676
);
7777

78-
const eCoordinate = new SingleFieldInspectorComponent(
78+
const eCoordinate = new InputFieldInspectorComponent(
7979
"E",
8080
"number",
8181
() => this.position.z,

canvas_editor/static/js/editor/objects/lightSource.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { DeleteLightSourceCommand } from "deleteCommands";
33
import { DuplicateLightSourceCommand } from "duplicateCommands";
44
import {
55
HeaderInspectorComponent,
6-
SingleFieldInspectorComponent,
6+
InputFieldInspectorComponent,
77
SelectFieldInspectorComponent,
88
InspectorComponent,
99
} from "inspectorComponents";
@@ -80,7 +80,7 @@ export class LightSource extends CanvasObject {
8080
this,
8181
);
8282

83-
this.#numberOfRaysComponent = new SingleFieldInspectorComponent(
83+
this.#numberOfRaysComponent = new InputFieldInspectorComponent(
8484
"Number of rays",
8585
"number",
8686
() => this.numberOfRays,
@@ -108,7 +108,7 @@ export class LightSource extends CanvasObject {
108108
},
109109
);
110110

111-
this.#distributionMeanComponent = new SingleFieldInspectorComponent(
111+
this.#distributionMeanComponent = new InputFieldInspectorComponent(
112112
"Mean",
113113
"number",
114114
() => this.distributionMean,
@@ -118,7 +118,7 @@ export class LightSource extends CanvasObject {
118118
-Infinity,
119119
);
120120

121-
this.#distributionCovarianceComponent = new SingleFieldInspectorComponent(
121+
this.#distributionCovarianceComponent = new InputFieldInspectorComponent(
122122
"Covariance",
123123
"number",
124124
() => this.distributionCovariance,

0 commit comments

Comments
 (0)