-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
phrasemaker.js: Bug Fix, Linting and Prettify. #2848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
js/widgets/phrasemaker.js
Outdated
if (condition === "graphicsblocks") { | ||
if (blockLabel === "forward" || blockLabel === "back") { | ||
for (let i = 0; i < forwardBackLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} else if (blockLabel === "right" || blockLabel === "left") { | ||
for (let i = 0; i < leftRightLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} else if (blockLabel === "setheading") { | ||
for (let i = 0; i < setHeadingLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} else if (blockLabel === "setpensize") { | ||
for (let i = 0; i < setPenSizeLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} else { | ||
for (let i = 0; i < setLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} | ||
} else if (condition === "synthsblocks") { | ||
for (let i = 0; i < valueLabel.length; i++) { | ||
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this code dry.
Declare a variable => labelLength
put if else according to your conditions and set value of noteStored
Use for loop once.
Something like:-
let labelLength = 0;
if (condition === "graphicsblocks") {
if (blockLabel === "forward" || blockLabel === "back") {
labelLength = forwardBackLabel.length
}
} else if (blockLabel === "right" || blockLabel === "left") {
labelLength = leftRightLabel.length;
}
...
//loop just once
for (let i = 0; i < labelLength; i++) {
this._pitchWheel.navItems[i].navigateFunction = __enterArgValue;
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make code dry
Check this last commit. @vaibhavdaren |
Please review this @meganindya @walterbender. |
I believe I should close this PR as the linting is done in #2885 by @ksraj123 and those changes have been merged and add this bug fix and some changes making the code DRY in patch #2891. Please share your feedback regarding this @walterbender @meganindya. |
Issue Reference: #2609
Bug:
On clicking the
divide
button to divide a current more in half, the widget produces the messageClick on the table to add notes.
every time and also re-positions the widget.screen-capture.1.mp4