refactor: improve code quality and robustness#5643
refactor: improve code quality and robustness#5643Ashutoshx7 wants to merge 7 commits intosugarlabs:masterfrom
Conversation
- index.html: implement retry mechanism for doSearch to prevent initialization errors - js/turtle-painter.js: export Painter class to global window scope for loader compatibility - js/widgets/modewidget.js: implement dependency injection pattern to reduce global state usage
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
|
✅ All Jest tests passed! This PR is ready to merge. |
|
✅ All Jest tests passed! This PR is ready to merge. |
|
do search was removed from index.html so that change (and large diff) is not needed here. |
doSearch was removed from index.html upstream, so the tryDoSearch retry logic and related changes are no longer needed. This reverts index.html to match upstream master.
|
✅ All Jest tests passed! This PR is ready to merge. |
ready for review |
|
I am a bit confused as to what I am looking at. The changes only vaguely resemble the description in your PR |
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
updated the pr description and resolved merge conflict |
2 participants
Overview
This PR improves code quality and robustness in two core areas of the codebase.
1. Dependency Injection in
ModeWidget(js/widgets/modewidget.js)Problem
ModeWidgetrelied on deeply nested property chains throughout the class:this.activity.logo.*this.activity.turtles.*this.activity.blocks.*This created tight coupling to the activity global, making the code harder to test and maintain.
Solution
Refactored the constructor to resolve dependencies upfront from the activity object into local instance properties:
this.logo,this.turtles,this.blocks,this.storagethis.hideMsgs,this.textMsg,this.errorMsg,this.refreshCanvasAn optional
depsparameter allows explicit dependency injection in tests, while theactivityreference is preserved for backward compatibility.Result
this.logo.synth.trigger(...)instead ofthis.activity.logo.synth.trigger(...))2. Global Export for
Painter(js/turtle-painter.js)Problem
The
Painterclass was exported only via CommonJS (module.exports), but the RequireJS shim in the browser expected it onwindow.Solution
Added a conditional
window.Painter = Painterexport so the class is:Testing
ModeWidgettests pass (modewidget.test.js+__tests__/modewidget.test.js)ModeWidgetopens, plays modes, saves, and rotates correctly in the browserPainterloads correctly via RequireJS once again