Skip to content
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.

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

Create hooks exercise for useState and useEffect #18

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

hanyuone
Copy link
Collaborator

@hanyuone hanyuone commented Jun 7, 2024

See title. The exercise is a very basic idle game, which uses TypeScript for enhanced capabilities with the code editor. Also includes a section which describes why useState and useEffect are necessary, and how to use both hooks.

Please review TP directors 🥺

@JeydinNewWon
Copy link
Collaborator

JeydinNewWon commented Aug 31, 2024

Coding Portion

  • The exercise has been improved a lot - I can do the first and two exercises pretty quickly. I think the code is a tick here.
  • A minor improvement might be adding more documentation as to what a "Highlight" is, just for convenience so no need to flick between screens.
  • Exercise 3 could potentially be a bit difficult for trainees since some might not have the intuition to use setInterval, but at the same time, idk how you'd make it simpler. Consider marking Exercise 3 as an "extension" whereas Exercise 2 completion is for sufficient competency.

Markdown

  • I would like a bit more context as to what an idle game is, and what kind of thing we are creating, lol.
  • Exercise 2's guide seems a bit wishy washy. Each of the components/functions have details of what they are, but it would be better if there was more explanation on their role, i.e. Highlight creates the colouring effect of the code snippet.
  • I like the addition of a small .mp4 to show an example of a working final product! However, I felt this was pretty misleading when it came to incorporating Exercise 3. Perhaps you could include a second .mp4 for Exercise 3 to demonstrate the Programmers and Forum Answers increasing the number of lines every second (i.e. increasing line count without clicking).

@hanyuone
Copy link
Collaborator Author

hanyuone commented Sep 1, 2024

TODOs from @JeydinNewWon's comment:

  • Add documentation on types and
  • Split task 3 up, turn some tasks into extensions
  • Add explainer on idle games
  • Clear up task 2 walkthrough
  • Fix sample solution, make new video demonstrating automatic resource gain

@TAS-scorchedshadow
Copy link
Collaborator

Exercise Portion

  • Exercise is way easier to understand with the jsx comments
  • Reorganisation of the starter code also makes the exercise easier to start
  • Agree with Jayden that the highlight portion could be introduced better. Perhaps add a sentence explicitly stating that highlighting needs to be performed for part 2
  • In App.jsx ln44: The TODOs is a little vague. Consider adding a small section that explains the overall goal of the task e.g. "generates new line of code and displays it in place of the old one". I like the specificity to which code fragments need to be used so keep what you have in!

Markdown Portion

  • The useState and useEffect page is solid overall. Explanations look good. There's some areas where terminology can be made simpler to accommodate 1st year coders.
    • ln4. "React Engine" (I believe we can just use React, not sure what the specific terminology we use for the render)
    • ln54. global mutable variables and namespace (either explain "global mutable & namespace" or use the vague plain English versions)
    • ln 171. idiom
  • Pure Function explanation was clear
  • Exercise 2 needs a clearer explanation on what the overall goal is. "Get a line of code and display it with highlighting". The rest of the instructions were easy to understand.
  • Appreciate the additions of the screenshots.

@hanyuone
Copy link
Collaborator Author

hanyuone commented Sep 1, 2024

UPDATE: made the exercises more accessible (thank you @JeydinNewWon @TAS-scorchedshadow!!). I don't think I'll have time to work on this so the only blocker I can see from the existing comments is to bugfix the auto-increment not working properly. If someone can do this that would be greatly appreciated 🙏🙏🙏

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.

3 participants