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

Solution #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Solution #16

wants to merge 4 commits into from

Conversation

NikitaUshenko
Copy link


componentDidMount() {
this.setState(prevState => ({
visibleTodos: [...prevState.allTodos],

Choose a reason for hiding this comment

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

чому б одразу не записати в initialTodos = {
id: 1,
title: 'Hello',
completed: false,
}

і присвоїти його одразу в allTodos і visibleTodos

src/App.js Outdated
completed: false,
}],
}));
this.setFilter(this.state.currentFilter);

Choose a reason for hiding this comment

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

не факт що після виклику setState this.state.currentFilter буде вже оновлений раджу визвати цю функцію колбеком для setState({...}, () => this.setFilter(....)). Це буде гарантією того що стейт уже змінився

src/App.js Outdated
todo.completed = !todo.completed;
}

return 0;

Choose a reason for hiding this comment

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

Не можу зрозуміти. Поясни буть ласка чому тут 0?

Copy link
Author

Choose a reason for hiding this comment

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

Бо лінтер чекає повернення значення у функції, це просто заглушка

src/App.js Outdated
}

handleToggle = (id) => {
this.setState((prevState) => {

Choose a reason for hiding this comment

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

для чого визивати setState якщо не оновлюєш стейт?

src/App.js Outdated
prevState.allTodos.map((todo) => {
if (todo.id === id) {
// eslint-disable-next-line no-param-reassign
todo.completed = !todo.completed;

Choose a reason for hiding this comment

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

модифікує дані в масиві: {
...todo,
completed: !todo.completed
}

src/App.js Outdated

handleToggle = (id) => {
this.setState((prevState) => {
prevState.allTodos.map((todo) => {

Choose a reason for hiding this comment

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

для чого map якщо нікуди не присвоююш новий масив?

src/App.js Outdated
<span className="todo-count">
{this.state.allTodos.filter(todo => (
todo.completed === false
)).length}

Choose a reason for hiding this comment

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

this.state.allTodos.filter(todo => ! todo.completed).length

src/App.js Outdated
<button
type="button"
className="clear-completed"
style={{ display: 'block' }}

Choose a reason for hiding this comment

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

це краще зробити в css


class NewTodo extends React.Component {
state = {
input: '',

Choose a reason for hiding this comment

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

старайся називати буз привязки до елементу а до сутності. Не буже ж ти називати input1, input2 якщо буде потрібно дописати декілька інпутів

function TodoList({ todos, handleToggle, handleRemove }) {
return (
<>
<input type="checkbox" id="toggle-all" className="toggle-all" />

Choose a reason for hiding this comment

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

для чого цей чекбокс якщо він не нисе ніякої логіки?

src/App.js Outdated
}

removeTodo = (id) => {
this.setState(prevState => ({
allTodos: prevState.allTodos.filter(todo => id !== todo.id),
}));
this.setFilter(this.state.currentFilter);
}), () => this.setFilter(this.state.currentFilter));

Choose a reason for hiding this comment

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

Делай деструктуризацию this.state

}

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

const title = this.state.input;
const title = this.state.todo;

Choose a reason for hiding this comment

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

Деструктуризация this.state

@@ -8,12 +8,12 @@ function TodoItem({ todo, handleToggle, handleRemove }) {
<input
type="checkbox"
className="toggle"
id="todo-3"
id={`todo-${todo.id}`}

Choose a reason for hiding this comment

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

Незачем приписывать к id префикс todo-, он от этого более уникальным не становится

onChange={event => handleToggleAll(event)}
checked={todos.length === todos
.filter(todo => todo.completed === true)
.length}

Choose a reason for hiding this comment

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

checked={todos.every(todo => todo.completed)}

@vkryvytskyy
Copy link

Учти комментарии

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants