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

feat(curriculum): add first debugging project to JS curriculum #54622

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

Conversation

jdwilkin4
Copy link
Contributor

Summary of changes

Currently the new JS curriculum does not have a debugging section like the old one did.
Originally there was going to be project to cover basic debugging, there were issues on how to deal with teaching syntax errors with the current editor. So that project was scrapped.

But we still need to teach debugging. This small project teaches campers how to resolve common issues and read common errors messages like TypeError and ReferenceError . They learn this by helping camperbot fix their code.

Checklist:

@jdwilkin4 jdwilkin4 added the new javascript course These are for issues dealing with the new JS curriculum label May 2, 2024
@jdwilkin4 jdwilkin4 self-assigned this May 2, 2024
@github-actions github-actions bot added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: i18n language translation/internationalization. Often combined with language type label labels May 2, 2024
@jdwilkin4 jdwilkin4 marked this pull request as ready for review May 2, 2024 15:29
@jdwilkin4 jdwilkin4 requested a review from a team as a code owner May 2, 2024 15:29
@jdwilkin4 jdwilkin4 added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label May 2, 2024
You should not call the `changeBackgroundColor` function. You should instead assign the function reference to the `onclick` property.

```js
assert.strictEqual(btn.onclick, changeBackgroundColor);
Copy link
Member

Choose a reason for hiding this comment

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

Not necessarily blocking, buuuuuut this means that the valid code won't pass:

btn.onclick = (() => changeBackgroundColor());

Not sure if we want to account for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am having a hard time get both solutions to pass with the updated tests
Each additional test I try, I can only get the btn.onclick = changeBackgroundColor to pass but not the other solution.

@Ksound22
Copy link
Member

Ksound22 commented May 6, 2024

This is a priority for the day. I will drop my suggestions before the end of the day.

Copy link
Member

@Ksound22 Ksound22 left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀

@jdwilkin4
Copy link
Contributor Author

jdwilkin4 commented May 6, 2024

Spoke with Oliver and he will look into seeing how we can teach syntax errors and have hints show when syntax errors are present.

For now, I will label this as blocked until I hear back from Oliver

@jdwilkin4 jdwilkin4 added status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels May 6, 2024

A `ReferenceError` is thrown when a non-existent variable is referenced. In this case, it looks like CamperBot is trying to use `math` but JavaScript doesn't have a `math` object.

Fix CamperBot's error in the `math.random()` line. Then open up the console to see the error message disappear.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Fix CamperBot's error in the `math.random()` line. Then open up the console to see the error message disappear.
Fix CamperBot's error in the `math.random()` line. Then open up the console to see if the error message disappeared or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should change this to if the error message disappeared or not. Because that implies after making the change, the error message might still be there. But if done correctly, the error message shouldn't be there.

We could change this to be

Fix CamperBot's error in the `math.random()` line and open up the console again.

Because the goal is to show campers that there is an error and get them in the habit of checking console statements. Then have them fix the error. and get them in the habit of checking the console again.

Copy link
Contributor

Choose a reason for hiding this comment

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

the code linting looks off, seems as if the css and html tags are not highlighted. can you please check from your end if some syntax is missing or not ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and couldn't find anything.
I even copied and pasted the same HTML and CSS from other steps and it is the exact same.
So I don't know why the syntax looks different here.


# --hints--

You should not call the `changeBackgroundColor` function. You should instead assign the function reference to the `onclick` property.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You should not call the `changeBackgroundColor` function. You should instead assign the function reference to the `onclick` property.
You should not call the `changeBackgroundColor` function, instead assign the function reference to the `onclick` property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new javascript course These are for issues dealing with the new JS curriculum platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants