learn-co-curriculum/react-forms-lab

React Controlled Components Lab

Overview

In this lab, you'll write and use controlled components, and write validation for form components.

Controlled Components

Now that we know how to handle form elements in React and how to set up controlled components, it's time to put that knowledge to the test. This lab is fairly extensive, but you'll use many core React concepts here that will surface again and again. Time to get some practice in!

General notes for this lab:

• Most of the DOM is pre-written for you in these exercises. Please do not remove anything that is already in the file — most of it is necessary to ensure that the tests run correctly.

• When instructed to save a value in the state, it doesn't matter what key you use, as long as it's in there!

TwitterMessage

1. Open the components/TwitterMessage.js file.

2. This component takes one prop: maxChars which is a number — the maximum amount of characters a message can have. This prop is being passed in from the App component with the value 280.

3. You'll find an <input type="text"> in this component. Make this a controlled component by adding the attributes to the <input> element. Its value should be saved in the component's state and should update on every change to the input.

4. Show the remaining characters in the component. It doesn't matter how you render it, as long as the number is correct. No need to guard against input that is too long — you can let the counter reach negative values.

LoginForm

1. Open the components/LoginForm.js file.

2. This component takes one prop: handleLogin which is a function — this function is called when the form is being submitted. By default, this function currently just includes console.log. That will allow you to see if the form is functioning correctly when working in your browser.

3. You'll find two inputs in this component: <input type="text"> and <input type="password">. Make this a controlled component by adding the necessary attributes to these inputs. The input values should be saved to the component's state on every change.

4. An example of an input would look like:

   <input
     id="username"
     type="text"
     name="username"
     value={this.state.username}
     onChange={this.handleInputChange}
   />

5. Remember that you can retrieve the input name and value of an event.target from the JS event.

6. Add the necessary event handler to the <form> element in order to call the onSubmit callback prop.

7. The onSubmit callback prop should only be called if both fields are filled in (with any value).

Note: In the starter code are id attributes - these are used in the tests, so make sure to leave them as they are.

Resources

• React Forms

react-forms-lab's Issues

trouble with `yarn start` requires changing dependencies

I'm not sure what causes this, but there's definitely an error with dependencies in this lab.

Good news: this error message provides clear instructions to fix it.

Fix: "delete ‘webpack’ and ‘webpack-dev-server’ from the ‘devDependencies’ fixes it on my end"

yarn start
yarn run v1.15.2
$ react-scripts start

There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.

The react-scripts package provided by Create React App requires a dependency:

  "webpack": "4.28.3"

Don't try to install it manually: your package manager does it automatically.
However, a different version of webpack was detected higher up in the tree:

  /Users/sgeluso/Junkyard/react-forms-lab-seattle-web-career-021819/node_modules/webpack (version: 3.12.0) 

Manually installing incompatible versions is known to cause hard-to-debug issues.

If you would prefer to ignore this check, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That will permanently disable this message but you might encounter other issues.

To fix the dependency tree, try following the steps below in the exact order:

  1. Delete package-lock.json (not package.json!) and/or yarn.lock in your project folder.
  2. Delete node_modules in your project folder.
  3. Remove "webpack" from dependencies and/or devDependencies in the package.json file in your project folder.
  4. Run npm install or yarn, depending on the package manager you use.

In most cases, this should be enough to fix the problem.
If this has not helped, there are a few other things you can try:

  5. If you used npm, install yarn (http://yarnpkg.com/) and repeat the above steps with it instead.
     This may help because npm has known issues with package hoisting which may get resolved in future versions.

  6. Check if /Users/sgeluso/Junkyard/react-forms-lab-seattle-web-career-021819/node_modules/webpack is outside your project directory.
     For example, you might have accidentally installed something in your home folder.

  7. Try running npm ls webpack in your project folder.
     This will tell you which other package (apart from the expected react-scripts) installed webpack.

If nothing else helps, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That would permanently disable this preflight check in case you want to proceed anyway.

P.S. We know this message is long but please read the steps above :-) We hope you find them helpful!

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Extra words in this sentence

You can remove "is currently just" from this sentence, and put "a" before console.log:
"By default, this function is currently just includes console.log that will allow you to see if the form is functioning correctly when working in your browser."

Thank you.

Tests fail if requirements are met for login form

Lines 62 and 99 of LoginForm-test.js test that it is true that:

"The default form action is not being prevented when the form is submitted" (line 62)

"The onSubmit prop is not being called" (line 99)

But the lab requires that the default form action IS prevented and the onSubmit prop IS called, resulting in conflicts that make the tests unpassable. Changing these to false gets tests passing. Confirmed in the browser.

Typo on 5th requirement for LoginForm

"inpiut" should be "input"
"Remember that you can retrieve the inpiut name and value of an event.target from the JS event"

Tests on poemWriter throw false positves

@gj

It is possible to pass all the test yet crash the server one using the poemWriter a second time.

On solutions that do not check to see with the poem is three lines long before validating the amount of word per line. So on any poem tested after the first initial poem the server will crash when code simillar to this is ran

`function countWords(line) {
return line.split(' ').filter(l => l).length;
}

function isValidPoem(poem) {
const poemLines = poem.split('\n').filter(l => l);
const isRightAmountOfLines = poemLines.length === 3;
const hasRightAmountOfWords = countWords(poemLines[0]) === 5 && countWords(poemLines[1]) === 3 && countWords(poemLines[2]) === 5;
return isRightAmountOfLines && hasRightAmountOfWords;
}`

It seems when poem is passed the second time it has now length therefore crashing the server.

Conflicting test

In the README is says that a poem with leading/trailing whitespace is invalid, but the test says:

"should not show an error when the poem is valid but has extra whitespace"

Can't run tests without running npm install --save raf

I opened this lab and ran learn immediately. Without making any changes to my cloned repo, the tests threw a bunch of errors and wouldn't run.

A quick google search led to the following solution:
npm install --save raf
Then as your first import in entry point, before React:
import 'raf/polyfill';

but it appears that just installing --save raf was enough to get the tests running.

Leading/Trailing whitespace. Whats the story?

It seems like leading whitespace is a problem in the poem, but trailing is supposed to be ok.
But in the test, const VALID_POEM_WITH_EXTRA_WHITESPACE = ${"word ".repeat(5)}
${"word ".repeat(3)}
${"word ".repeat(5)} ;
the valid poem should not trip the error message.

Well, the valid poem over here has leading whitespace on the second line. backspacing that line and saving the test makes it fine. but in that case the only additional functionality the test requires is that multiple spaces as opposed to one are allowed as trailing.

I recommend changing line 17 of the poem test by backspacing to the start of the line. because the readme is quite explicitly ruling out leading whitespace.

Simulated event causes opinionated test

In src/components/TwitterMessage.js, a common pattern to make form-control change event handlers abstract is to write them as such:

handleChange = (event) => {
    this.setState({
      [event.target.id]: event.target.value
    });
  }

Then, by adding id attributes to your form elements that match the keys of your state, the handleChange function is now reusable.

This causes the following test to fail, since the simulated event does not contain any information about the event target's actual attributes:

describe("<TwitterMessage />", () => {
  describe("Saving input value in state", () => {
    it("should update the state when typing", () => {
      const wrapper = shallow(<TwitterMessage maxChars={140} />);
      const event = { target: { value: "f" } };
      wrapper.find("input").simulate("change", event);
      expect(wrapper.find("input").props().value).to.deep.equal(
        event.target.value,
        "The input value is not being updated when it changes"
      );
    });
  });

Adding an id attribute to the JSX in TwitterMessage.js as well as the simulated event will encourage setting the state more abstractly rather than explicitly with setState({ message: event.target.value})

ReadMe has Controlled Components Header Twice.

environment broken

npm start doesn't work

Rewrite

Several spelling and grammar issues in this lesson.

"Make this a controlled component by adding the necessary props to these inputs. Their values should be saved in the component's state." - needs to be pluralized properly

"Remember that you can retrieve the inpiut name and value of an event.target from the JS event." - def not how you spell input.

Probably a couple more too.

Unclear instructions?

7. The onSubmit callback prop should only be called if both fields are filled in (with any value).

This makes it sound like they need a ternary in the onSubmit property of the button to only call this.handleSubmit if the conditions are met. I don't THINK that's the intention but I could be wrong. If I'm correct, I would suggest wording it like this:

7. The callback prop passed to the component should only be called if both fields are filled in (with any value).

Lack of instruction or pre-existing knowlegde

Canvas Link

https://learning.flatironschool.com/courses/1883/assignments/125690?module_item_id=259623

Concern

This lab makes use of an OnChange event that has not previously been covered. While I do not expect to be spoon-fed every possible line of code I am on this course to spend my time efficiently learning from experts, yet far too much time is spend finding things that have not been covered or are only covered after fumbling through a lab involving them. I didn't intend to pay for a course and end up teaching myself.

Additional Context

No response

Suggested Changes

No response

Tests pass without meeting requirements

This test: Should call the `onSubmit` callback prop when the form is being submitted passes even if I never call onSubmit.

Passing code that doesn't meet the requirements:

class LoginForm extends React.Component {
  constructor() {
    super();

    this.state = {
      username: '',
      password: ''
    };
  }

  inputChange = e => this.setState({[e.target.id]: e.target.value});

  handleSubmit = event => {
    event.preventDefault()
  }

  render() {
    return (
      <form onSubmit={this.handleSubmit}>
        <div>
          <label>
            Username
            <input id="username" name="username" type="text" onChange = {event => this.inputChange(event)}/>
          </label>
        </div>
        <div>
          <label>
            Password
            <input id="password" name="password" type="password"
            onChange = {event => this.inputChange(event)}
            />
          </label>
        </div>
        <div>
          <button type="submit">Log in</button>
        </div>
      </form>
    );
  }
}

export default LoginForm;

Suggestion: Add a test for `console.log` output

As I was writing the code that calls the handleLogin prop on the LoginForm component, I wrote this by mistake:

this.props.handleLogin(this.state.username, this.state.password);

That didn't work, which makes sense. No matter what values I used for the username and password, the code above always caused the console to display Logging in undefined with password undefined.

The problem is that this code actually passes the tests. This is because none of them check the console.log output. Therefore, I would like to suggest adding a LoginForm test that does something like this:

1. Fill in the LoginForm's username input field with something like randomUser.
2. Fill in the LoginForm's password input field with something like mypassword.
3. Click the Log in button.
4. Check that Logging in randomUser with password mypassword was correctly displayed in the console.

This suggestion is probably for an edge case, so please feel free to take it or leave it.

missing devDependencie

package.json "devDependencies" is missing "sinon": "1.17.7"
ran into errors running bundle until I added "sinon".
Then I ran npm install again and tried npm run bundle and it worked.

readme refs handleSubmit() -- tests and lab use handleLogin

according to issue 35, lab was updated? but mine is not. It still reference handleSubmit instead of handle login.

Tests pass in 'dumb' events which cause problems

The tests pass in events which only have a single property, target, which itself only has a single value, value:

    it('should call the `onSubmit` callback prop when the form is being submitted', function () {
      // ...
      wrapper.find('#test-username').simulate('change', { target: { value: 'johndoe' } });
      wrapper.find('#test-password').simulate('change', { target: { value: 'supersecret' } });
      // ...
      expect(spy.calledOnce).toBeTruthy('The `onSubmit` prop is not being called exactly once');
    });

So, if you try to use the event for anything but the event.target.value, your tests will fail in a very non-obvious way. For example, one obvious (but failing) way of solving this problem is by adding a name to your input like this:

          <label>
            Username
            <input
              id="test-username"
              name="username"
              type="text"
              value={this.state.username}
              onChange={this.handleChange} />
          </label>

...with a state whose shape looks like this:

    this.state = {
      username: '',
      password: ''
    };

...then you might write a change handler like this:

  handleChange(event) {
    const target = event.target;
    this.setState({
      [target.name]: target.value
    });
  }

That's gonna fail, because event.target.name is undefined.

Similarly, if you do this other obvious solution, where you use the id instead of name, it still fails:

    this.state = {
      'test-username': '',
      'test-password': ''
    };

...and:

  handleChange(event) {
    const target = event.target;
    this.setState({
      [target.id]: target.value
    });
  }

That will fail as well. Instead, in order to pass the tests, you have to do the dirtier way of passing an extra argument bound to the change handler:

  handleChange(field, event) {
    const target = event.target;
    this.setState({
      [field]: target.value
    });
  }

...and:

          <label>
            Username
            <input
              id="test-username"
              type="text"
              value={this.state.username}
              onChange={this.handleChange.bind(this, 'username')} />
          </label>

That's totally unnecessary. All the data you need should be on the event object already. And this.handleChange already is bound to this in the constructor, which means it's not even saving that step.

inpuit --> input in LoginForm

THIS LAB!!!?

please provide more details on what is expected of this lab. Where do we get the character amount?
How do we know to set it to message?

Controlled Compotents heading used twice

"Controlled Components" is a section heading that appears twice in a row

Error: Method “simulate” is only meant to be run on a single node. 0 found instead.

When a student received this error on the lab, the reason is because the ID in the component does not match the ID passed into the find function in the test.

Able to pass tests without meeting all instruction requirements.

I was able to get the PoemWriter function to pass without actually meeting the requirements listed in the instructions. Basically, if you just check whether or not there are three lines you can circumvent all other solution requirements.

maxChars set to 10

Unless we look closely at the test, we wouldn't know the maxChars is 10 vs. the normal 140 Twitter uses. This should be stated in the directions.

Webpack Error

There was a pretty nasty bug that had to do with webpack version. You'll know it when you run into it.

Solution:

1. Delete webpack and webpack-dev-server from package.json. Save.
2. Delete package-lock.json
3. Delete the hidden folder, "node_modules". In your terminal run rm -rf node_modules
4. npm install

This corrected the webpack versioning issue for me.

confusing instructions

Literally no where does it say what it wants you to do. I can get from implications in the text that it wants you to count character count?
Specifically point two in TWITTERMESSAGE

"handleSubmit" function does not exist

"handleSubmit" prop function is actually called handleLogin

Prop for LoginForm component incorrect

In the readme it states:

This component takes one prop: onSubmit which is a function — this function is called when the form is being submitted. By default, this function currently just includes console.log. That will allow you to see if the form is functioning correctly when working in your browser.

After chatting with a tech coach we decided it should say: This component takes one prop: handleLogin

This is what is actually being passed to the component in App.js: