Skip to content

Hertanto | PTBC2-1#43

Open
hertantoirawan wants to merge 56 commits intorocketacademy:mainfrom
hertantoirawan:master
Open

Hertanto | PTBC2-1#43
hertantoirawan wants to merge 56 commits intorocketacademy:mainfrom
hertantoirawan:master

Conversation

@hertantoirawan
Copy link

Please fill out the survey before submitting the pull request. Thanks!

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

How many hours did you spend on this assignment?
Probably 30-40 hours

Please fill in one error and/or error message you received while working on this assignment.

  • Uncaught ReferenceError: module is not defined

What part of the assignment did you spend the most time on?
UI (css) and getting the game as complete as possible (bug fixing, edge cases, icon/image editing, etc)

Comfort Level (1-5):
4 because there are several parts of my project that I know only at the basics level

Completeness Level (1-5):
4 because I didn't really do More Comfortable

What did you think of this deliverable?
Fun

Is there anything in this code that you feel pleased about?
UI, which is a challenge for me but I pushed through it, and also the extra stuff that I was able to implement to make the game more fun and complete

Copy link

@upieez upieez left a comment

Choose a reason for hiding this comment

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

What went well

  • Great documentation in your README.md and your Wiki
  • Frequent commits
  • Great use of third-party resources for your design (NES.css) and your hints
  • It is mobile responsive
  • Great use of JSDoc for your functions
  • Your function names are well written and easy to read and understand
  • Excellent customization (through the options menu) and the secret codes

What can be improved

  • Consider splitting the functions in your script.js into different files for better separation of concerns
  • Having unit tests is good but for this particular project, the time spent on understanding and implementing it could have been dedicated on other areas such as working on the "More Comfortable"
  • Creative use of background image for your cards but they are usually not very good for accessibility. Instead, you could consider using img tags with an alt text to help with accessibility
  • Consider omitting comments that is already self-explanatory so as not to clutter your codebase

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.

2 participants