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

React PropTypes Challenge Unclear #39204

Closed
jonathanihm opened this issue Jul 8, 2020 · 6 comments · Fixed by #39682
Closed

React PropTypes Challenge Unclear #39204

jonathanihm opened this issue Jul 8, 2020 · 6 comments · Fixed by #39682
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.

Comments

@jonathanihm
Copy link
Contributor

https://www.freecodecamp.org/learn/front-end-libraries/react/use-proptypes-to-define-the-props-you-expect

In the instructions and unit tests are ambiguous with the word "required".

It states:
"The Items component should include a propTypes check that requires quantity to be a number."

This is unclear as "requires" could mean that this check just simply needs to exist for the test to pass or that quantity should be a number that is required.

I suggest either changing the above wording, or adding an additional unit test:
"The Items component should include a quantity propType that is required."

@jonathanihm jonathanihm added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. labels Jul 8, 2020
@naomi-lgbt
Copy link
Member

image

I think the lesson instructions are pretty clear, but I do agree that the test case wording is ambiguous. I would propose that the test message be edited to mirror this instruction.

@RandellDawson
Copy link
Member

RandellDawson commented Jul 8, 2020

I do not think the test text should just be a duplicate of the instruction. That would be redundant. Instead, we need to rewrite the text to reflect was is being tested. See below for the current testString prettified:

assert((function() {
  const noWhiteSpace = getUserInput('index').replace(/ /g, '');
  return noWhiteSpace.includes('quantity:PropTypes.number.isRequired') && noWhiteSpace.includes('Items.propTypes=');
})());

The important part of the test is in the return statement. We are validating that we see the text "quantity:PropTypes.number.isRequired" and "Items.propTypes='" after removing whitespaces from the code. There probably is a better testString that could be written also.

@naomi-lgbt
Copy link
Member

I cannot think of a more effective testString, personally. But for the test text, perhaps something like "The Items component should include a propTypes check to require a value for quantity and ensure that value is a number."

@jonathanihm
Copy link
Contributor Author

@huyenltnguyen huyenltnguyen added status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. and removed status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. labels Jul 21, 2020
@jp-sauve
Copy link

I agree that the text should be updated. I just got caught up by the unclear instructions. The way it is, I thought that what was 'required' was that quantity be a number. It doesn't suggest to me that a quantity is required.

@RandellDawson RandellDawson added first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. and removed status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. labels Sep 24, 2020
@RandellDawson
Copy link
Member

Opening this PR for contributions. The test text should be changed to:

The Items component should include a propTypes check to require a value for quantity and ensure that its value is a number.

RandellDawson added a commit that referenced this issue Sep 25, 2020
* In React propTypes challenge, improve test text

Fixes #39204

* Remove code tags around "number"

Co-authored-by: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>

Co-authored-by: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants