Skip to content

Conversation

@illicitonion
Copy link
Member

For starter code, the IDs try to be good examples for how to name
elements.

For example values as block parameters, the IDs try to be relevant to
some example, but not necessarily the first example.

Fixes #26.

For starter code, the IDs try to be good examples for how to name
elements.

For example values as block parameters, the IDs try to be relevant to
_some_ example, but not necessarily the first example.
@illicitonion illicitonion requested a review from gregdyke January 24, 2022 21:11
Copy link
Collaborator

@gregdyke gregdyke left a comment

Choose a reason for hiding this comment

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

Instead of modifying the code in blockly-dom.js (which should probably also have better - but more generic - names), we should modify the starter xml in each of the toolboxes in index.html (and the generic starter xml at the end of the file)

<li>Start with an empty unordered html list (<code>&lt;ul&gt;</code>) and a button to add apples to the list (<code>&lt;button&gt;</code>) in the static html.
<ol>
<li>for example: <code class="start_code">&lt;ul id="list"&gt;&lt;/ul&gt;<br>&lt;button id="button"&gt;add an apple&lt;/button&gt;</code></li>
<li>for example: <code class="start_code">&lt;ul id="fruits"&gt;&lt;/ul&gt;<br>&lt;button id="add-apple-button"&gt;add an apple&lt;/button&gt;</code></li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

are dashed ids a good practice (I know classnames are often dashed)? I would probably underscore them, to avoid teaching identifiers with - in them

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a super strong preference - I started dashing somewhat at random, so changed a few _s to -s because I'd done most of them, but happy to change them all to _s if you'd prefer :)

</p>
<ol>
<li>Start with an empty list (where we will display our rolls), a place to put a total, and a few buttons (<code class="start_code">&lt;p&gt;So far you have rolled:&lt;/p&gt;<br>&lt;ul id="list"&gt;&lt;/ul&gt;<br>&lt;button id="button_roll"&gt;Roll the dice&lt;/button&gt;<br>&lt;p&gt;Total: &lt;span id="total"&gt;0&lt;/span&gt;. &lt;span id="info"&gt;Keep playing!&lt;/span&gt;&lt;/p&gt;<br>&lt;button id="button_remove"&gt;Remove the last roll&lt;/button&gt;<br>&lt;button id="button_restart"&gt;Start again&lt;/button&gt;</code></li>
<li>Start with an empty list (where we will display our rolls), a place to put a total, and a few buttons (<code class="start_code">&lt;p&gt;So far you have rolled:&lt;/p&gt;<br>&lt;ul id="previous-rolls"&gt;&lt;/ul&gt;<br>&lt;button id="roll-button"&gt;Roll the dice&lt;/button&gt;<br>&lt;p&gt;Total: &lt;span id="total"&gt;0&lt;/span&gt;. &lt;span id="info"&gt;Keep playing!&lt;/span&gt;&lt;/p&gt;<br>&lt;button id="remove-roll-button"&gt;Remove the last roll&lt;/button&gt;<br>&lt;button id="restart-button"&gt;Start again&lt;/button&gt;</code></li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

pretty sure all these ids are referred to later

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't see any... Do you see any specifically?

@gregdyke
Copy link
Collaborator

Also woop, you picked up a ticket!!!!!

@gregdyke
Copy link
Collaborator

huh. I googled and I learnt something about hyphenated ids and why :)

@illicitonion
Copy link
Member Author

Instead of modifying the code in blockly-dom.js (which should probably also have better - but more generic - names), we should modify the starter xml in each of the toolboxes in index.html (and the generic starter xml at the end of the file)

Things are looking about right in the rendered UI for me... What's the purpose of blockly-dom.js vs the stuff in index.html?

@gregdyke
Copy link
Collaborator

So blockly-dom is intended as a standalone thing, useful beyond CYF - I think it should have boring generic variable names by default (/be improved to have cleverer variable names), especially as we can write the variables in the custom toolboxes (and even have different default variable names per exercise)

For the rest of the change, unfortunately, probably better wait til we've moved to .md files and then port

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

better ids throughout

3 participants