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

aasdasd #2682

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

aasdasd #2682

Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ const pattern = /^((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=+$,\w]+@)?[A-Za-z0-9.-]+|(
- 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_movies-list-add-form/) and add it to the PR description.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://sinner1993.github.io/react_movies-list-add-form/) and add it to the PR description.
18 changes: 16 additions & 2 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,29 @@ import './App.scss';
import { MoviesList } from './components/MoviesList';
import { NewMovie } from './components/NewMovie';
import moviesFromServer from './api/movies.json';
import { useState } from 'react';

type Movie = {
title: string;
description: string;
imgUrl: string;
imdbUrl: string;
imdbId: string;
};

export const App = () => {
const [films, setFilms] = useState<Movie[]>(moviesFromServer as Movie[]);

Choose a reason for hiding this comment

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

Ensure that moviesFromServer is correctly structured as an array of Movie objects. The type assertion as Movie[] assumes that the data matches the Movie type, which can lead to runtime errors if the structure is incorrect. Verify the structure of moviesFromServer to prevent potential issues.

const handleAdd = (newMovie: Movie): void => {
setFilms(prevMovies => [...prevMovies, newMovie]);
};

return (
<div className="page">
<div className="page-content">
<MoviesList movies={moviesFromServer} />
<MoviesList movies={films} />
</div>
<div className="sidebar">
<NewMovie /* onAdd={(movie) => {}} */ />
<NewMovie onAdd={handleAdd} />
</div>
</div>
);
Expand Down
93 changes: 84 additions & 9 deletions src/components/NewMovie/NewMovie.tsx
Original file line number Diff line number Diff line change
@@ -1,37 +1,112 @@
import { useState } from 'react';
import { FormEvent, useState } from 'react';
import { TextField } from '../TextField';

export const NewMovie = () => {
type Movie = {
title: string;
description: string;
imgUrl: string;
imdbUrl: string;
imdbId: string;
};

type NewMovieProps = {
onAdd: (newMovie: Movie) => void;
};

export const NewMovie: React.FC<NewMovieProps> = ({ onAdd }) => {
// Increase the count after successful form submission
// to reset touched status of all the `Field`s
const [count] = useState(0);
const [title, setTitle] = useState<string>('');
const [description, setDescription] = useState<string>('');
const [imgUrl, setImgUrl] = useState<string>('');
const [imdbUrl, setImdbUrl] = useState<string>('');
const [imdbId, setImdbId] = useState<string>('');
const buttonEnabled = title && imgUrl && imdbUrl && imdbId ? true : false;

Choose a reason for hiding this comment

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

The buttonEnabled logic should ensure that all required fields are not only filled but also trimmed of spaces. Consider using trim() on each field to prevent submissions with only whitespace.

const handleSubmit = (event: FormEvent<HTMLFormElement>): void => {
event.preventDefault();

const form = event.target as HTMLFormElement;
const formData = new FormData(form);

const data = Object.fromEntries(formData.entries()) as {
[key: string]: string;
};

const newMovie: Movie = {
title: data.title,
description: data.description,
imgUrl: data.imgUrl,
imdbUrl: data.imdbUrl,
imdbId: data.imdbId,
};

onAdd(newMovie);

form.reset();

setTitle('');
setDescription('');
setImgUrl('');
setImdbUrl('');
setImdbId('');
};

return (
<form className="NewMovie" key={count}>
<form
className="NewMovie"
key={count}
onSubmit={event => handleSubmit(event)}
>
<h2 className="title">Add a movie</h2>

<TextField
name="title"
label="Title"
value=""
onChange={() => {}}
value={title}
required
onChange={setTitle}
/>
Comment on lines +67 to +69

Choose a reason for hiding this comment

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

The onChange handler should update the state with the current input value. Ensure that the setTitle function is correctly updating the title state variable. This applies to all TextField components.


<TextField name="description" label="Description" value="" />
<TextField
name="description"
label="Description"
value={description}
onChange={setDescription}
/>

<TextField name="imgUrl" label="Image URL" value="" />
<TextField
name="imgUrl"
label="Image URL"
value={imgUrl}
required
onChange={setImgUrl}
/>
Comment on lines +82 to +84

Choose a reason for hiding this comment

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

Ensure that the setImgUrl function is correctly updating the imgUrl state variable. The onChange handler should receive the current input value.


<TextField name="imdbUrl" label="Imdb URL" value="" />
<TextField
name="imdbUrl"
label="Imdb URL"
value={imdbUrl}
required
onChange={setImdbUrl}
/>
Comment on lines +90 to +92

Choose a reason for hiding this comment

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

Ensure that the setImdbUrl function is correctly updating the imdbUrl state variable. The onChange handler should receive the current input value.


<TextField name="imdbId" label="Imdb ID" value="" />
<TextField
name="imdbId"
label="Imdb ID"
value={imdbId}
required
onChange={setImdbId}
/>
Comment on lines +98 to +100

Choose a reason for hiding this comment

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

Ensure that the setImdbId function is correctly updating the imdbId state variable. The onChange handler should receive the current input value.


<div className="field is-grouped">
<div className="control">
<button
type="submit"
data-cy="submit-button"
className="button is-link"
disabled={!buttonEnabled}
>
Add
</button>
Expand Down
14 changes: 10 additions & 4 deletions src/components/TextField/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type Props = {
label?: string;
placeholder?: string;
required?: boolean;
onChange?: (newValue: string) => void;
onChange: (newValue: string) => void;
};

function getRandomDigits() {
Expand All @@ -16,11 +16,11 @@ function getRandomDigits() {

export const TextField: React.FC<Props> = ({
name,
value,
label = name,
value,
placeholder = `Enter ${label}`,
required = false,
onChange = () => {},
onChange,
}) => {
// generate a unique id once on component load
const [id] = useState(() => `${name}-${getRandomDigits()}`);
Expand All @@ -29,6 +29,10 @@ export const TextField: React.FC<Props> = ({
const [touched, setTouched] = useState(false);
const hasError = touched && required && !value;

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

return (
<div className="field">
<label className="label" htmlFor={id}>
Expand All @@ -37,6 +41,7 @@ export const TextField: React.FC<Props> = ({

<div className="control">
<input
name={name}
type="text"
id={id}
data-cy={`movie-${name}`}
Expand All @@ -45,8 +50,9 @@ export const TextField: React.FC<Props> = ({
})}
placeholder={placeholder}
value={value}
onChange={event => onChange(event.target.value)}
onChange={event => handleOnChange(event)}
onBlur={() => setTouched(true)}
required={name !== 'description' && !value}

Choose a reason for hiding this comment

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

The required attribute logic is incorrect. It should be set based on the required prop passed to the component, not the value. This ensures that the field is marked as required only if the required prop is true.

/>
</div>

Expand Down
Loading