Create Visualization class for Artist
#20534
Conversation
apps/src/turtle/turtle.js
Outdated
| this.isDrawingAnswer_ = false; | ||
| this.isPredrawing_ = false; | ||
|
|
||
| // This flag is used to draw a version of code (either user code or solution | ||
| // code) that nornamlizes patterns and stickers to always use the "first" | ||
| // option, so that validation can be agnostic | ||
| this.shouldDrawNormalized_ = false; | ||
|
|
||
| this.visualization = new Visualization(); |
There was a problem hiding this comment.
is it okay for canvases to be created on the DOM here, rather than in afterInject_?
apps/src/turtle/turtle.js
Outdated
| this.x = DEFAULT_X; | ||
| this.y = DEFAULT_Y; | ||
| this.heading = 0; | ||
| this.penDownValue = true; |
There was a problem hiding this comment.
penDownValue seems like it should still live on Artist
|
My goal is to extract a new version of the class I have here, but a little more carefully than what I did in the other PR. Maybe |
# Conflicts: # apps/src/turtle/skins.js # apps/src/turtle/turtle.js
|
Ok, I think this is ready for review! Sorry about the huge diff 😖. It should only be moving code from Artist -> Visualization class and ref updates. |
|
Followup diff to extract the module is here: visualization-class...artist-module. |
# Conflicts: # apps/src/turtle/turtle.js
There was a problem hiding this comment.
This looks great! I'd love to see some follow-up work to look at all the places the artist class is doing a bunch of work directly on the Visualization and see how much of that can be yanked out into more tightly-controlled methods on Visualization itself, but this is awesome for the initial extraction
Replaces PR #20280.