Skip to content

Commit 98e75ec

Browse files
authored
ref: hoist DOM element queries outside loop (sugarlabs#7619)
* refactor(musickeyboard): hoist DOM element queries outside loop * fix(musickeyboard): correct variable names and add test for _createKeyboard
1 parent a283857 commit 98e75ec

2 files changed

Lines changed: 37 additions & 8 deletions

File tree

js/widgets/__tests__/musickeyboard.test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,39 @@ describe("MusicKeyboard add-row submenu", () => {
209209
expect(keyboard._exitWheel.navItems[1].enabled).toBe(false);
210210
expect(keyboard._exitWheel.navItems[0].sliceSelectedAttr.cursor).toBe("pointer");
211211
});
212+
test("creates keyboard without throwing and sets up idContainer", () => {
213+
global.PITCHES3 = ["C", "D", "E", "F", "G", "A", "B"];
214+
global.SHARP = "♯";
215+
global.FLAT = "♭";
216+
const keyboard = new MusicKeyboard({
217+
canvas: { width: 800, height: 600 },
218+
getStageScale: () => 1
219+
});
220+
221+
keyboard.keyboardDiv = document.createElement("div");
222+
document.body.appendChild(keyboard.keyboardDiv);
223+
224+
keyboard.displayLayout = [
225+
{ noteName: "hertz", noteOctave: 392, blockNumber: 100001 },
226+
{ noteName: "drum", noteOctave: 436, blockNumber: 100002 },
227+
{ noteName: "C", noteOctave: 4, blockNumber: 100003 },
228+
{ noteName: "C#", noteOctave: 4, blockNumber: 100004 },
229+
{ noteName: "Db", noteOctave: 4, blockNumber: 100005 }
230+
];
231+
keyboard.layout = keyboard.displayLayout;
232+
keyboard.noteNames = [];
233+
keyboard.octaves = [];
234+
keyboard.loadHandler = jest.fn();
235+
keyboard.addKeyboardShortcuts = jest.fn();
236+
237+
expect(() => keyboard._createKeyboard()).not.toThrow();
238+
expect(keyboard.idContainer.length).toBeGreaterThan(0);
239+
240+
document.body.removeChild(keyboard.keyboardDiv);
241+
delete global.PITCHES3;
242+
delete global.SHARP;
243+
delete global.FLAT;
244+
});
212245
});
213246

214247
describe("MusicKeyboard widgetWindow.onclose & event cleanup", () => {

js/widgets/musickeyboard.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2959,11 +2959,13 @@ function MusicKeyboard(activity) {
29592959
let myrow2Id = 0;
29602960
let myrow3Id = 0;
29612961

2962-
let parenttbl, parenttbl2, el, newel, newel2, nname;
2962+
let parenttbl = document.getElementById("myrow");
2963+
let parenttbl2 = document.getElementById("myrow2");
2964+
let el, newel, newel2, nname;
2965+
29632966
for (let p = 0; p < this.displayLayout.length; p++) {
29642967
// If the blockNumber is null, don't add a label.
29652968
if (this.displayLayout[p].noteName > FAKEBLOCKNUMBER) {
2966-
parenttbl2 = document.getElementById("myrow2");
29672969
newel2 = document.createElement("td");
29682970
newel2.setAttribute("id", "blackRow" + myrow2Id.toString());
29692971
if (
@@ -3005,7 +3007,6 @@ function MusicKeyboard(activity) {
30053007
newel2.style.visibility = "hidden";
30063008
parenttbl2.appendChild(newel2);
30073009
} else if (this.displayLayout[p].noteName === "drum") {
3008-
parenttbl = document.getElementById("myrow");
30093010
newel = document.createElement("td");
30103011
newel.style.textAlign = "center";
30113012
newel.setAttribute("id", "whiteRow" + myrowId.toString());
@@ -3035,7 +3036,6 @@ function MusicKeyboard(activity) {
30353036
newel.style.zIndex = "100";
30363037
parenttbl.appendChild(newel);
30373038
} else if (this.displayLayout[p].noteName === "hertz") {
3038-
parenttbl = document.getElementById("myrow");
30393039
newel = document.createElement("td");
30403040
newel.style.textAlign = "center";
30413041
newel.setAttribute("id", "hertzRow" + myrow3Id.toString());
@@ -3068,7 +3068,6 @@ function MusicKeyboard(activity) {
30683068
this.displayLayout[p].noteName.includes(SHARP) ||
30693069
this.displayLayout[p].noteName.includes("#")
30703070
) {
3071-
parenttbl2 = document.getElementById("myrow2");
30723071
newel2 = document.createElement("td");
30733072
newel2.setAttribute("id", "blackRow" + myrow2Id.toString());
30743073
newel2.style.textAlign = "center";
@@ -3125,7 +3124,6 @@ function MusicKeyboard(activity) {
31253124
this.displayLayout[p].noteName.includes(FLAT) ||
31263125
this.displayLayout[p].noteName.includes("b")
31273126
) {
3128-
parenttbl2 = document.getElementById("myrow2");
31293127
newel2 = document.createElement("td");
31303128
// elementid2 = document.getElementsByTagName("td").length;
31313129
newel2.setAttribute("id", "blackRow" + myrow2Id.toString());
@@ -3191,7 +3189,6 @@ function MusicKeyboard(activity) {
31913189
newel2.style.zIndex = "200";
31923190
parenttbl2.appendChild(newel2);
31933191
} else {
3194-
parenttbl = document.getElementById("myrow");
31953192
newel = document.createElement("td");
31963193
// elementid = document.getElementsByTagName("td").length;
31973194

@@ -3242,7 +3239,6 @@ function MusicKeyboard(activity) {
32423239
parenttbl.appendChild(newel);
32433240
}
32443241
}
3245-
parenttbl = document.getElementById("myrow");
32463242
newel = document.createElement("td");
32473243
parenttbl.appendChild(newel);
32483244
newel.style.textAlign = "center";

0 commit comments

Comments
 (0)