Reid187
Reid187

Reputation: 63

React - Function unaware of state when called from keydown event

I've been working on learning React, and in doing so built a todo app using class components. Recently I have been working to make a copy of the todo app using functions and hooks instead of classes.

Having refactored the code everything appears to be working correctly aside from one use case.

CASE 1: When typing into the input and clicking "Add" button to call addItem() the new todo item adds as expected.

CASE 2: When typing into the input and hitting enter to trigger an event handler which calls addItem() the value of newItem is always the same as its initial value.

I can't for the life of me figure out why addItem() behaves differently when called from the click of the "Add" button versus the press of the "Enter" key.

TodoAppContainer.js

import React, { useState, useEffect } from 'react';
import TodoList from './TodoList';
import TodoForm from './TodoForm';
import { GenerateID } from './generateId';

export default function TodoListContainer(props) {

    const [newItem, setNewItem] = useState('New Todo');
    const [items, setItems] = useState([{
        name: 'Build Todo List App',
        done: true,
        key: GenerateID.next().value
    }]);

    const handleKeyDown = e => {
        if (e.key === 'Enter') addItem();
    };

    const handleChange = ({ target }) => {
        console.log("handleChange");
        // capture text from input field
        const text = target.value;

        // update state value for "newItem"
        setNewItem(text);
    };

    const addItem = () => {
        console.log("addItem");
        // exit early if there is no item
        if (!!!newItem.trim()) return;

        // build new item to add
        const itemToAdd = {
            name: newItem,
            done: false,
            key: GenerateID.next().value
        };

        // update state with new item
        setItems(prevItems => [itemToAdd, ...prevItems]);

        // clear text for new item
        setNewItem('');
    };

    const completeItem = key => {
        console.log('completeItem');
        // create new copy of state items
        const updatedItems = [...items];

        // get the index of the item to update
        const index = updatedItems.findIndex(v => v.key === key);

        // toggle the done state of the item
        updatedItems[index].done = !updatedItems[index].done;

        // update the state
        setItems(updatedItems);
    };

    const removeItem = key => {
        console.log('removeItem');
        // create copy of filtered items
        const filteredItems = items.filter(v => v.key !== key);

        // update the state of items
        setItems(filteredItems);
    }

    // get count of items that are "done"
    const getTodoCount = () => items.filter(v => v.done).length;

    useEffect(() => {
        document.addEventListener('keydown', handleKeyDown);
        return () => document.removeEventListener('keydown', handleKeyDown);
    }, []);

    return (
        <section className='todo-section'>
            <TodoForm
                newItem={newItem}
                handleChange={handleChange}
                addItem={addItem}
            />
            <TodoList
                items={items}
                count={getTodoCount()}
                onClick={completeItem}
                onRemove={removeItem}
            />
        </section>
    );
}

TodoForm.js

import React from 'react';
import PropTypes from 'prop-types';

export default function TodoForm(props) {
    const { newItem, handleChange, addItem } = props;
    return (
        <div className='todo-form'>
            <input type='text' value={newItem} onChange={handleChange} />
            <button onClick={addItem}>Add</button>
        </div>
    )
}

TodoForm.propTypes = {
    newItem: PropTypes.string.isRequired,
    addItem: PropTypes.func.isRequired,
    handleChange: PropTypes.func.isRequired
};

TodoList.js

import React from 'react';
import PropTypes from 'prop-types';

export default function TodoList(props) {
    const { items, count, onClick, onRemove } = props;
    const shrug = '¯\\_(ツ)_/¯';
    const shrugStyles = { fontSize: '2rem', fontWeight: 400, textAlign: 'center' };

    const buildItemHTML = ({ key, name, done }) => {
        const className = done ? 'todo-item done' : 'todo-item';
        return (
            <li className={className} key={key}>
                <span className='item-name' onClick={() => onClick(key)}>{name}</span>
                <span className='remove-icon' onClick={() => onRemove(key)}>&#10006;</span>
            </li>
        );
    };

    return (
        <div>
            <p style={{ margin: 0, padding: '0.75em' }}>{count} of {items.length} Items Complete!</p>
            <ul className='todo-list'>
                {items.length ? items.map(buildItemHTML) : <h1 style={shrugStyles}>{shrug}<br />No items here...</h1>}
            </ul>
        </div>
    );
};

TodoList.propTypes = {
    count: PropTypes.number.isRequired,
    items: PropTypes.array.isRequired,
    onClick: PropTypes.func.isRequired,
    onRemove: PropTypes.func.isRequired
};

Upvotes: 3

Views: 888

Answers (3)

Cybershadow
Cybershadow

Reputation: 1167

This is happening because the you are adding the event listener inside useEffect and at that time the value of newItem is your initial newItem.

To make it work , you can add newItem to dependecy array to update the event listeners, every time newItem updates.

useEffect(() => {
        document.addEventListener('keydown', handleKeyDown);
        return () => document.removeEventListener('keydown', handleKeyDown);
    }, [newItem]);

This however is just a solution but not the recommended solution. Adding event listeners this way is not very React-ish.

Instead of writing event listeners in useEffect.You should instead bind an event like this

export default function TodoForm(props) {
    const { newItem, handleChange, addItem ,handleKeyDown} = props;
    return (
        <div className='todo-form'>
            <input type='text' 
                value={newItem} 
                onChange={handleChange} 
                onKeyDown={handleKeyDown}// this is new
             />
            <button onClick={addItem}>Add</button>
        </div>
    )
}

And don't forget to add it in the parent component

<TodoForm
                newItem={newItem}
                handleChange={handleChange}
                handleKeydown={handleKeydown}//this is new
                addItem={addItem}
            />

Upvotes: 1

Ron B.
Ron B.

Reputation: 1540

This is a classic case of stale closure. You add the keydown event to the document only once when the component mounts, and it remembers the function only on the first render, when the state is an the default string.

One way to solve it is add newItem as a dependency to your useEffect, you can also use the onKeyDown prop on the input of your new todo, but that might not act as you expect (e.g. the input needs to be focused for the event to fire)

Upvotes: 1

PixAff
PixAff

Reputation: 329

try to add this to your input:

<input type='text' value={newItem} onChange={handleChange} onKeyDown={(event) => {
          if (event.key === "Enter") {
            addItem();
          }
        }}/>

And delete the eventListener from your useEffect() - actually you can delete the whole useEffect() since it only tries to handle your eventListener...

Upvotes: 1

Related Questions