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

fix(curriculum): fix tests for step 6 of the todo project #54626

Merged
merged 2 commits into from May 7, 2024

Conversation

Deep512
Copy link
Contributor

@Deep512 Deep512 commented May 2, 2024

Checklist:

Closes #54625

Updated the regex to handle the only unhandled answer:

openTaskFormBtn.addEventListener('click', () => {
      taskForm.classList.toggle('hidden')
});

Issue addressed in the forum

Post fix screenshot:
Screenshot 2024-05-02 at 11 20 43 PM

@github-actions github-actions bot added the scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. label May 2, 2024
@Deep512 Deep512 marked this pull request as ready for review May 2, 2024 17:56
@Deep512 Deep512 changed the title fix(curriculum): correct the regex string in step 6 to handle all cases fix(curriculum): fix tests for step 6 of the todo project May 3, 2024
@Deep512
Copy link
Contributor Author

Deep512 commented May 3, 2024

@ilenia-magoni @gikf Please review the PR

@camperbot
Copy link
Contributor

We realize you're looking to get help as soon as possible. Rather than pinging someone directly, which can be considered rude, would you mind joining our Discord and asking your question there? Someone might be more readily available to help.

Copy link
Member

@naomi-lgbt naomi-lgbt left a comment

Choose a reason for hiding this comment

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

Hmmm I wonder if we should just click the button in the tests and then assert the modal has the class?

@Deep512
Copy link
Contributor Author

Deep512 commented May 3, 2024

Hmmm I wonder if we should just click the button in the tests and then assert the modal has the class?

That sure is a valid alternative solution.

However, considering that the regex testing was already implemented in the codebase, I chose to address the issue by updating the regex to handle the edge case.

Let me know if you think we should still update the test logic.

@ilenia-magoni
Copy link
Contributor

Please stop pinging people to require a review, it's the second time you are being asked to avoid doing that please

@Deep512
Copy link
Contributor Author

Deep512 commented May 4, 2024

Please stop pinging people to require a review, it's the second time you are being asked to avoid doing that please

I am extremely sorry @ilenia-magoni, didn't intend to cause any inconvenience. Wasn't aware of the process of review here and might have missed the bot message.
I'll ensure to use the discord platform in the future

@jdwilkin4 jdwilkin4 added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP new javascript course These are for issues dealing with the new JS curriculum labels May 7, 2024
…a-structures-22/learn-localstorage-by-building-a-todo-app/64e4e78a7ea4a168de4e6a38.md

Co-authored-by: Jessica Wilkins  <67210629+jdwilkin4@users.noreply.github.com>
@naomi-lgbt naomi-lgbt added status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. and removed status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels May 7, 2024
@jdwilkin4 jdwilkin4 merged commit 9ac9576 into freeCodeCamp:main May 7, 2024
22 checks passed
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 scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix tests for step 6 of todo project
5 participants