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.

Sign up for GitHub

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

add task solution #2676

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

add task solution #2676

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3da44ab
add task solution
Mariagosp Dec 12, 2024
0f0226e
add task solution
Mariagosp Dec 12, 2024
332b8d6
add task solution
Mariagosp Dec 13, 2024
aa01a1b
add task solution
Mariagosp Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ loaded and show them using `TodoList` (check the code in the `api.ts`);
- Implement a solution following the [React task guideline](https://github.com/mate-academy/react_task-guideline#react-tasks-guideline).
- Use the [React TypeScript cheat sheet](https://mate-academy.github.io/fe-program/js/extra/react-typescript).
- Open one more terminal and run tests with `npm test` to ensure your solution is correct.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://<your_account>.github.io/react_dynamic-list-of-todos/) and add it to the PR description.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://Mariagosp.github.io/react_dynamic-list-of-todos/) and add it to the PR description.
76 changes: 70 additions & 6 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,66 @@
/* eslint-disable max-len */
import React from 'react';
import React, { useEffect, useState } from 'react';
import 'bulma/css/bulma.css';
import '@fortawesome/fontawesome-free/css/all.css';

import { TodoList } from './components/TodoList';
import { TodoFilter } from './components/TodoFilter';
import { TodoModal } from './components/TodoModal';
import { Loader } from './components/Loader';
import { Todo } from './types/Todo';
import { getTodos } from './api';
import { Filters } from './types/Filters';

export const App: React.FC = () => {
const [todos, setTodos] = useState<Todo[]>([]);
const [query, setQuery] = useState<string>('');
const [status, setStatus] = useState<string>('all');
Copy link

Choose a reason for hiding this comment

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

  1. If you pass the type as (string, number, boolean) - the compiler understands what type it is and you don't need to type it.
  2. u created a generic for this. Use it in ur state
Suggested change
const [query, setQuery] = useState<string>('');
const [status, setStatus] = useState<string>('all');
const [query, setQuery] = useState('');
const [status, setStatus] = useState<Filters>(Filters.ALL);

const [selectedTodoId, setSelectedTodoId] = useState<number | null>(null);

const [loadingTodos, setLoadingTodos] = useState(true);
Copy link

Choose a reason for hiding this comment

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

for boolean variables use prefixes (is , has )

Suggested change
const [loadingTodos, setLoadingTodos] = useState(true);
const [isLoadingTodos, setIsLoadingTodos] = useState(true);


const filteredTodos = todos
.filter(todo => todo.title.toLowerCase().includes(query.toLowerCase()))
.filter(todo => {
if (status === Filters.ALL) {
return true;
}

return status === Filters.COMPLETED ? todo.completed : !todo.completed;
});
Copy link

Choose a reason for hiding this comment

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

instead use only 1 filter (logic in the middle of the filter method callback)


const selected = todos.find(todo => todo.id === selectedTodoId);
Copy link

Choose a reason for hiding this comment

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

selected what?change naming for this variable


const onQueryChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setQuery(event.target.value);
};

const onReset = () => {
setQuery('');
};

const onOpenModal = (todo: Todo) => {
setSelectedTodoId(todo.id);
};

const onCloseModal = () => {
setSelectedTodoId(null);
};

const onStatusChange = (event: React.ChangeEvent<HTMLSelectElement>) => {
setStatus(event.target.value as Filters);
};
Copy link

Choose a reason for hiding this comment

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

use handle prefix for handle functions(in parent components) - naming-convention

Suggested change
const onQueryChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setQuery(event.target.value);
};
const onReset = () => {
setQuery('');
};
const onOpenModal = (todo: Todo) => {
setSelectedTodoId(todo.id);
};
const onCloseModal = () => {
setSelectedTodoId(null);
};
const onStatusChange = (event: React.ChangeEvent<HTMLSelectElement>) => {
setStatus(event.target.value as Filters);
};
const handleQueryChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setQuery(event.target.value);
};
const handleReset = () => {
setQuery('');
};
const handleOpenModal = (todo: Todo) => {
setSelectedTodoId(todo.id);
};
const handleCloseModal = () => {
setSelectedTodoId(null);
};
const handleStatusChange = (event: React.ChangeEvent<HTMLSelectElement>) => {
setStatus(event.target.value as Filters);
};


useEffect(() => {
setLoadingTodos(true);

getTodos()
.then(data => {
setTodos(data);
})
.finally(() => setLoadingTodos(false));
}, []);

return (
<>
<div className="section">
Expand All @@ -17,18 +69,30 @@ export const App: React.FC = () => {
<h1 className="title">Todos:</h1>

<div className="block">
<TodoFilter />
<TodoFilter
query={query}
onQueryChange={onQueryChange}
onReset={onReset}
onStatusChange={onStatusChange}
/>
</div>

<div className="block">
<Loader />
<TodoList />
{loadingTodos && todos.length === 0 && <Loader />}
{!loadingTodos && (
<TodoList
todos={filteredTodos}
selectedTodoId={selectedTodoId}
onOpenModal={onOpenModal}
/>
)}
Copy link

Choose a reason for hiding this comment

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

Will be more readable and better

Suggested change
{loadingTodos && todos.length === 0 && <Loader />}
{!loadingTodos && (
<TodoList
todos={filteredTodos}
selectedTodoId={selectedTodoId}
onOpenModal={onOpenModal}
/>
)}
{(loadingTodos && !todos.length) ? (
<Loader />
) : (
<TodoList
todos={filteredTodos}
selectedTodoId={selectedTodoId}
onOpenModal={onOpenModal}
/>
)}

</div>
</div>
</div>
</div>

<TodoModal />
{selectedTodoId && selected && (
<TodoModal todo={selected} onCloseModal={onCloseModal} />
)}
</>
);
};
78 changes: 50 additions & 28 deletions src/components/TodoFilter/TodoFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,52 @@
export const TodoFilter = () => (
<form className="field has-addons">
<p className="control">
<span className="select">
<select data-cy="statusSelect">
<option value="all">All</option>
<option value="active">Active</option>
<option value="completed">Completed</option>
</select>
</span>
</p>
import { Filters } from '../../types/Filters';

<p className="control is-expanded has-icons-left has-icons-right">
<input
data-cy="searchInput"
type="text"
className="input"
placeholder="Search..."
/>
<span className="icon is-left">
<i className="fas fa-magnifying-glass" />
</span>
type Props = {
query: string;
onQueryChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
onReset: () => void;
onStatusChange: (event: React.ChangeEvent<HTMLSelectElement>) => void;
};

<span className="icon is-right" style={{ pointerEvents: 'all' }}>
{/* eslint-disable-next-line jsx-a11y/control-has-associated-label */}
<button data-cy="clearSearchButton" type="button" className="delete" />
</span>
</p>
</form>
);
export const TodoFilter: React.FC<Props> = props => {
const { query, onQueryChange, onReset, onStatusChange } = props;

return (
<form className="field has-addons">
<p className="control">
<span className="select">
<select data-cy="statusSelect" onChange={onStatusChange}>
<option value={Filters.ALL}>All</option>
<option value={Filters.ACTIVE}>Active</option>
<option value={Filters.COMPLETED}>Completed</option>
</select>
Comment on lines +19 to +21
Copy link

Choose a reason for hiding this comment

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

Would be good to use map here

</span>
</p>

<p className="control is-expanded has-icons-left has-icons-right">
<input
data-cy="searchInput"
type="text"
className="input"
placeholder="Search..."
value={query}
onChange={onQueryChange}
/>
<span className="icon is-left">
<i className="fas fa-magnifying-glass" />
</span>

<span className="icon is-right" style={{ pointerEvents: 'all' }}>
{/* eslint-disable-next-line jsx-a11y/control-has-associated-label */}
Copy link

Choose a reason for hiding this comment

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

don't use inline styles

P.S. and yes, I know it was in the source code.
a programmer's job is to fix the bugs he sees)

{query && (
<button
data-cy="clearSearchButton"
type="button"
className="delete"
onClick={onReset}
/>
)}
</span>
</p>
</form>
);
};
164 changes: 70 additions & 94 deletions src/components/TodoList/TodoList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,100 +1,76 @@
import React from 'react';
import { Todo } from '../../types/Todo';
type Props = {
todos: Todo[];
onOpenModal: (todo: Todo) => void;
selectedTodoId: number | null;
};

export const TodoList: React.FC = () => (
<table className="table is-narrow is-fullwidth">
<thead>
<tr>
<th>#</th>
<th>
<span className="icon">
<i className="fas fa-check" />
</span>
</th>
<th>Title</th>
<th> </th>
</tr>
</thead>
export const TodoList: React.FC<Props> = props => {
const { todos, onOpenModal, selectedTodoId } = props;

<tbody>
<tr data-cy="todo" className="">
<td className="is-vcentered">1</td>
<td className="is-vcentered" />
<td className="is-vcentered is-expanded">
<p className="has-text-danger">delectus aut autem</p>
</td>
<td className="has-text-right is-vcentered">
<button data-cy="selectButton" className="button" type="button">
return (
<table className="table is-narrow is-fullwidth">
<thead>
<tr>
<th>#</th>
<th>
<span className="icon">
<i className="far fa-eye" />
<i className="fas fa-check" />
</span>
</button>
</td>
</tr>
<tr data-cy="todo" className="has-background-info-light">
<td className="is-vcentered">2</td>
<td className="is-vcentered" />
<td className="is-vcentered is-expanded">
<p className="has-text-danger">quis ut nam facilis et officia qui</p>
</td>
<td className="has-text-right is-vcentered">
<button data-cy="selectButton" className="button" type="button">
<span className="icon">
<i className="far fa-eye-slash" />
</span>
</button>
</td>
</tr>

<tr data-cy="todo" className="">
<td className="is-vcentered">1</td>
<td className="is-vcentered" />
<td className="is-vcentered is-expanded">
<p className="has-text-danger">delectus aut autem</p>
</td>
<td className="has-text-right is-vcentered">
<button data-cy="selectButton" className="button" type="button">
<span className="icon">
<i className="far fa-eye" />
</span>
</button>
</td>
</tr>
</th>
<th>Title</th>
<th> </th>
</tr>
</thead>

<tr data-cy="todo" className="">
<td className="is-vcentered">6</td>
<td className="is-vcentered" />
<td className="is-vcentered is-expanded">
<p className="has-text-danger">
qui ullam ratione quibusdam voluptatem quia omnis
</p>
</td>
<td className="has-text-right is-vcentered">
<button data-cy="selectButton" className="button" type="button">
<span className="icon">
<i className="far fa-eye" />
</span>
</button>
</td>
</tr>

<tr data-cy="todo" className="">
<td className="is-vcentered">8</td>
<td className="is-vcentered">
<span className="icon" data-cy="iconCompleted">
<i className="fas fa-check" />
</span>
</td>
<td className="is-vcentered is-expanded">
<p className="has-text-success">quo adipisci enim quam ut ab</p>
</td>
<td className="has-text-right is-vcentered">
<button data-cy="selectButton" className="button" type="button">
<span className="icon">
<i className="far fa-eye" />
</span>
</button>
</td>
</tr>
</tbody>
</table>
);
<tbody>
{todos.map(todo => (
<tr
data-cy="todo"
className={
selectedTodoId === todo.id ? 'has-background-info-light' : ''
Copy link

Choose a reason for hiding this comment

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

use classnames library for it. Use this library for all such cases

}
key={todo.id}
>
<td className="is-vcentered">{todo.id}</td>
<td className="is-vcentered">
{todo.completed && (
<span className="icon" data-cy="iconCompleted">
<i className="fas fa-check" />
</span>
)}
</td>
<td className="is-vcentered is-expanded">
<p
className={
todo.completed ? 'has-text-success' : 'has-text-danger'
}
>
{todo.title}
</p>
</td>
<td className="has-text-right is-vcentered">
<button
data-cy="selectButton"
className="button"
type="button"
onClick={() => onOpenModal(todo)}
>
<span className="icon">
<i
className={
selectedTodoId === todo.id
? 'far fa-eye-slash'
: 'far fa-eye'
}
/>
</span>
</button>
</td>
</tr>
))}
</tbody>
</table>
);
};
Loading
Loading