jmderas/towers-of-hanoi

The objective of the game is to move all the blocks from the left most column to the right most column.

towers-of-hanoi's Issues

Duplicative event handlers

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/main.js#L42-L123

It is likely you could use a single Event Listener on the parent container to account for all three towers. Three individual event listeners works just fine, but looking at the code I think of DRY (Don't Repeat Yourself) and feel that with a little time and attention you could write a more robust single function callback for the event handlers here.

commit f641413

One of the project requirements is to commit often throughout the project. There are not many commits for this project, and what is here gave me pause.

Commit f641413 adds a lot of new code, particularly in the JS. I do see that in Commit 7f226f8 you added your sources, which is absolutely what we need to see. I'm not pointing fingers, and I know you are a solid student from working with you and have no doubts about your personal integrity. But the frequent commit requirement is there for several reasons, and it is helpful to see many small commits over the course of large projects to give us insight into your process.

In the future I ask and advise you to make commits as you make progress so that we have a better idea what were stumbling blocks and how you solved them in your submission.

`class=hidden` missing quotations

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/index.html#L28

the attribute value "hidden" for the class needs to be wrapped in quotes. Browsers are helpful in that they can fill in missing characters like this, but if there was more than one class that fallback most likely would not work correctly so it is important to use proper syntax even if it "works" without it

Similar duplicative styles

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/style.css#L36-L65

For the rings, it could be useful to have a single .ring class and corresponding CSS styles for common properties like the background-image and height/width etc

No citation

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/style.css#L87-L120

You do a great job of listing the references in the Javascript, but the animation here looks like it's from somewhere. It's very important to always cite external sources, and I'm not pointing fingers I just think that if you did cite the animation I must have missed it.

Indentation

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/index.html#L16-L18

Indenting the code is awesome, but in this section and at least one other place the text is indented a few levels too far. Usually when writing the code line by line the editor (VSCode or otherwise) is good about inserting the correct indentation but if you're moving stuff around it can get wonky

`footer` is a better CSS selector

https://github.com/jmderas/towers-of-hanoi/blob/634ff23d5b6d5538bd2d34869852f7adc48ef549/style.css#L66

The div in the HTML is already a <footer>, so you can simply use footer instead of .foot as the CSS selector. One of the advantages of the semantic HTML elements