Change erick editor bitmap jack #598
Conversation
| subroutineName + | ||
| (Math.abs(currentJShift) / marginSaveFrames).toString(); | ||
| } | ||
| // Fixed name/signature requirements |
There was a problem hiding this comment.
This comment doesn't add anything
|
|
||
| if (document.getElementById("jackCode").checked) { | ||
| if (currentCodeMode === "jack") { | ||
| // Always generate a Jack function with this signature |
There was a problem hiding this comment.
Again, no value from this comment
| } else { | ||
| } else { |
There was a problem hiding this comment.
This change in whitespace looks sloppy
| newGrid[i][j] = grid[i][j]; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I like to keep a blank line after closing braces and new statements at the same block level
| document.getElementById("baseBottomLeft").disabled = true; | ||
| document.getElementById("codeTypeHeader").textContent = | ||
| "Generated Hack Assembly"; | ||
| // Update header text based on current mode |
There was a problem hiding this comment.
Comments should say how or why, not what
| <th align="left"> | ||
| <span id="codeTypeHeader">Generated Jack Code</span> | ||
| <form action="javascript:ResetSize()"> | ||
| <label for="inputWidth">Canvase Size: </label> |
There was a problem hiding this comment.
"Canvas", not "Canvase"; no trailing space. Labels should typically wrap their inputs, but in this case, there's two inputs, so the accessibility is going to suffer a bit. Maybe
<span>Canvas <label>Width <input id=... /></label> <label>Height <input id=.../></label> <label>Pixel Size <input.../></label></span>
| <th align="left">Bitmap</th> | ||
| <th align="left"> | ||
| <span id="codeTypeHeader">Generated Jack Code</span> | ||
| <form action="javascript:ResetSize()"> |
There was a problem hiding this comment.
This is a very atypical way to use forms and form actions. You should probably just add the behaviors as click handlers in JavaScript.
| </th> | ||
| <th align="left"> | ||
| <div class="tab-container"> | ||
| <button id="jackTab" class="tab-button active" onclick="SwitchToJack()">Generated Jack Code</button> |
There was a problem hiding this comment.
(Optional) Not the best treatment, a dropdown or button group would be better, but hey, this is something & we don't have a component library for this file.
| </p> | ||
| <p></p> | ||
|
|
||
| <p></p> |
There was a problem hiding this comment.
No need for an empty paragraph, is there?
| <table> | ||
| <tr> | ||
| <td align="center"> | ||
| <td align="center"> |
There was a problem hiding this comment.
Sloppy ending whitespace
It looks big , but mostly because I moved divs