Skip to content

Change erick editor bitmap jack #598

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.

Sign up for GitHub

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Change erick editor bitmap jack #598

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87f5fc4
feat(docs): Add bitmap editor user guide
renanbazinin Sep 18, 2025
7461983
Add bitmap editor user guide
renanbazinin Sep 18, 2025
04df146
Revert newline
DavidSouther Sep 18, 2025
5812101
UX changes at bitmap editor
renanbazinin Sep 18, 2025
8b1d8bb
button to looks like tab
renanbazinin Sep 18, 2025
d815144
Merge branch 'nand2tetris:main' into changeErickEditorBitmapJack
renanbazinin Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 109 additions & 180 deletions web/public/bitmap_editor.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,20 @@
baseJ: 0,
drawHeight: 0,
INVERT_MODE: false,
comments: true,
comments: true
};
var state = {};
try {
var savedState = localStorage.getItem(STATE_KEY);
if (savedState) {

var parsedState = JSON.parse(savedState);
state = Object.assign({}, defaults, parsedState);
} else {
state = defaults;
}
} catch (error) {
console.error(
"Error parsing state from localStorage. Using defaults.",
error,
);
console.error("Error parsing state from localStorage. Using defaults.", error);
state = defaults;
}
// initialize global variables from state
Expand Down Expand Up @@ -145,7 +143,7 @@
try {
Init();
} catch (error) {
console.log(error);
console.log(error)
Reset();
}
}
Expand Down Expand Up @@ -586,17 +584,12 @@
}

generateCode = document.getElementById("generatedCode");
let subroutineName = document.getElementById("subroutineName").value;
if (document.getElementById("changeNameOnShift").checked) {
subroutineName =
subroutineName +
(Math.abs(currentJShift) / marginSaveFrames).toString();
}
// Fixed name/signature requirements
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't add anything

let subroutineName = "draw";

if (document.getElementById("jackCode").checked) {
if (currentCodeMode === "jack") {
// Always generate a Jack function with this signature
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, no value from this comment

let subroutineType = "function";
if (document.getElementById("methodSubroutine").checked)
subroutineType = "method";

generateCode.value =
subroutineType +
Expand All @@ -621,7 +614,7 @@
}
}
generateCode.value = generateCode.value + "\treturn;\n}";
} else {
} else {
Comment on lines -624 to +617
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change in whitespace looks sloppy

// hack assembly
rowsOfWords = GetRowsFromWordColumns(colsOfWords);
generateCode.value = "(" + subroutineName + ")\n";
Expand Down Expand Up @@ -856,31 +849,35 @@
newGrid[i][j] = grid[i][j];
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to keep a blank line after closing braces and new statements at the same block level

saveGrid(newGrid);
saveState();
Init(); // init loads the new grid into grid
}
}

function ChangeName() {
if (document.getElementById("subroutineName").value == "")
document.getElementById("subroutineName").value = "draw";
GenerateCode(true);
}
function ChangeCodeType() {
if (document.getElementById("hackAssemblyCode").checked) {
document.getElementById("baseTopLeft").click();
document.getElementById("baseBottomLeft").disabled = true;
document.getElementById("codeTypeHeader").textContent =
"Generated Hack Assembly";
// Update header text based on current mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments should say how or why, not what

if (currentCodeMode === "hack") {
document.getElementById("hackTab").classList.add("active");
document.getElementById("jackTab").classList.remove("active");
} else {
document.getElementById("baseBottomLeft").disabled = false;
document.getElementById("codeTypeHeader").textContent =
"Generated Jack Code";
document.getElementById("jackTab").classList.add("active");
document.getElementById("hackTab").classList.remove("active");
}
GenerateCode();
}
// I made this global for ezier...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't even know what this comment is trying to say?

var currentCodeMode = "jack";

// convenience helpers for toggle controls
function SwitchToHack() {
currentCodeMode = "hack";
ChangeCodeType();
}
function SwitchToJack() {
currentCodeMode = "jack";
ChangeCodeType();
}
function UpdateMarginType() {
if (document.getElementById("fitToDrawing").checked) {
document.getElementById("marginSaveFramestd").hidden = false;
Expand All @@ -898,25 +895,83 @@
GenerateCode();
}
</script>

<!--
SPDX-FileCopyrightText: 2013 Golan Parashi
SPDX-FileCopyrightText: 2023 Erik Umble, <erikumble@gmail.com>
SPDX-License-Identifier: MIT
--></head>
</head>
<style>
.tab-button {
padding: 5px 10px;
border: 1px solid #ccc;
background-color: #f0f0f0;
cursor: pointer;
margin-bottom: -1px;
position: relative;
border-radius: 5px 5px 0 0;
}
.tab-button.active {
background-color: white;
border-bottom: 1px solid white;
}
</style>
<body onload="onLoad();">
<h1>Jack Bitmap Editor v2.9</h1>
<p>
See <a href="https://github.com/ErikUmble/JackBitmapEditor">here</a> for
updates, feature documentation, and contributions.

<p>Use this tool for drawing bitmap images and generating Hack or Jack code. <br/>
Then copy-pase the generated code into your Hack or Jack program. Change the generated label / function name, as needed.
</p>
<p></p>

<p></p>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for an empty paragraph, is there?

<table>
<thead>
<tr>
<th align="left">Bitmap</th>
<th align="left">
<span id="codeTypeHeader">Generated Jack Code</span>
<form action="javascript:ResetSize()">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very atypical way to use forms and form actions. You should probably just add the behaviors as click handlers in JavaScript.

<label for="inputWidth">Canvase Size: </label>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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>

<input
id="inputWidth"
placeholder="width"
maxlength="3"
size="3"
type="text"
value="48"
/>
x
<input
id="inputHeight"
placeholder="height"
maxlength="3"
size="3"
type="text"
value="32"
/>

<label for="pixelSize">Canvas pixel size: </label>
<input
id="pixelSize"
placeholder="px"
maxlength="2"
size="1"
type="text"
value="16"
/>

<button
title="Resize Canvas Preserving Upper-left Contents"
formaction="javascript:SetSize()"
>
Resize
</button>
<button
title="Reset Canvas Size, Settings, and Clear Contents"
formaction="javascript:Reset()"
>
Reset
</button>
</form>
</th>
<th align="left">
<div class="tab-container">
<button id="jackTab" class="tab-button active" onclick="SwitchToJack()">Generated Jack Code</button>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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.

<button id="hackTab" class="tab-button" onclick="SwitchToHack()">Generated Hack Assembly</button>
</div>
</th>
</tr>
<div id="style_stuff"></div>
Expand Down Expand Up @@ -1008,9 +1063,8 @@ <h1>Jack Bitmap Editor v2.9</h1>
</td>
<td align="center">
<input
checked
id="commentsOn"
onclick="GenerateCode()"
onclick="ToggleComments()"
type="checkbox"
/><label for="commentsOn">Comments On</label>
</td>
Expand All @@ -1024,58 +1078,7 @@ <h1>Jack Bitmap Editor v2.9</h1>
</tr>
</table>
</tr>
<tr>
<table>
<tr>
<td>
<form action="javascript:ResetSize()">
<label for="inputWidth">Canvase Size: </label>
<input
id="inputWidth"
placeholder="width"
maxlength="3"
size="3"
type="text"
value="48"
/>
x
<input
id="inputHeight"
placeholder="height"
maxlength="3"
size="3"
type="text"
value="32"
/>

<label for="pixelSize">Canvas pixel size: </label>
<input
id="pixelSize"
placeholder="px"
maxlength="2"
size="1"
type="text"
value="16"
/>

<button
title="Resize Canvas Preserving Upper-left Contents"
formaction="javascript:SetSize()"
>
Resize
</button>
<button
title="Reset Canvas Size, Settings, and Clear Contents"
formaction="javascript:Reset()"
>
Reset
</button>
</form>
</td>
</tr>
</table>
</tr>
<tr>

<table>
<tr>
<td align="center">
Expand Down Expand Up @@ -1121,99 +1124,25 @@ <h1>Jack Bitmap Editor v2.9</h1>
</tr>
</table>
</tr>
<tr>

<tr>
<table>
<tr>
<td align="center">
<td align="center">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sloppy ending whitespace

<label for="baseRowFrom">Base address:</label>
<input
checked
id="baseTopLeft"
name="baseRowFrom"
onclick="GenerateCode()"
type="radio"
/>
<input checked id="baseTopLeft" name="baseRowFrom" onclick="GenerateCode()" type="radio" />
<label for="baseTopLeft">Top Left</label>

<input
id="baseBottomLeft"
name="baseRowFrom"
onclick="GenerateCode()"
type="radio"
/>
<input id="baseBottomLeft" name="baseRowFrom" onclick="GenerateCode()" type="radio" />
<label for="baseBottomLeft">Bottom Left</label>
</td>
</tr>
</table>
</tr>
<tr>
<table>
<tr>
<td align="center">
<form action="javascript:ChangeName()">
<label for="subroutineName">Subroutine Name: </label>
<input
id="subroutineName"
placeholder="name"
type="text"
value="draw"
/>
<button>Generate Code</button>
</form>
</td>
<td align="center">
<input
checked
id="functionSubroutine"
name="subroutineType"
onclick="GenerateCode()"
type="radio"
/>
<label for="functionSubroutine">function</label>

<input
id="methodSubroutine"
name="subroutineType"
onclick="GenerateCode()"
type="radio"
/>
<label for="methodSubroutine">method</label>
</td>
<td align="center">
<input
id="changeNameOnShift"
onclick="GenerateCode()"
type="checkbox"
/><label for="pauseCode">Change name with horizontal shift</label>
</td>
</tr>
</table>
</tr>
<tr>
<table>
<tr>
<td align="center">
<input
checked
id="jackCode"
name="codeType"
onclick="ChangeCodeType()"
type="radio"
/>
<label for="jackCode">Jack</label>

<input
id="hackAssemblyCode"
name="codeType"
onclick="ChangeCodeType()"
type="radio"
/>
<label for="hackAssemblyCode">Hack Assembly</label>
</td>
</tr>
</table>
</tr>



</table>

<style>
.fileButton label {
display: block;
Expand Down Expand Up @@ -1354,4 +1283,4 @@ <h1>Jack Bitmap Editor v2.9</h1>
}
</script>
</body>
</html>
</html>