add task react_movies-list-add-form #2120

P-Nazar · 2024-01-20T10:59:30Z

etojeDenys · 2024-01-20T11:01:00Z

src/components/NewMovie/NewMovie.tsx

+  const [title, setTitle] = useState('');
+  const [description, setDescription] = useState('');
+  const [imgUrl, setImgUrl] = useState('');
+  const [imdbUrl, setimdbUrl] = useState('');
+  const [imdbId, setImdbId] = useState('');


we can combine it in one useState

const [newMovie, setNewMovie] = useState({ title: '', ... })

now we can also create only one handle function for all fields

const handleChange = (e: ...) => { const {name, value} = e.target; setMovie((prevMovie) => ({...prevMovie, [name]: value})) }

don't forget to specify name attr to input in TextField

VitaliyBondarenko1982

Almost done.
Please check a few comments.

VitaliyBondarenko1982 · 2024-01-20T13:19:18Z

src/components/NewMovie/NewMovie.tsx

-        label="Image URL"
-        value=""
+        value={newMovie.imgUrl}
+        onChange={e => handleChange(e)}


Suggested change

onChange={e => handleChange(e)}

onChange={handleChange}

it will be the same

VitaliyBondarenko1982 · 2024-01-20T13:19:34Z

src/components/NewMovie/NewMovie.tsx

-        label="Imdb URL"
-        value=""
+        value={newMovie.imdbUrl}
+        onChange={e => handleChange(e)}


Suggested change

onChange={e => handleChange(e)}

onChange={handleChange}

it will be the same

VitaliyBondarenko1982 · 2024-01-20T13:21:44Z

src/components/NewMovie/NewMovie.tsx

+            disabled={!newMovie.title
+              || !urlValidator(newMovie.imgUrl)
+              || !urlValidator(newMovie.imdbUrl)
+              || !newMovie.imdbId}


better to move in constant.

const isDisabled = !newMovie.title.trim() || !urlValidator(newMovie.imgUrl) || !urlValidator(newMovie.imdbUrl) || !newMovie.imdbId.trim()

Suggested change

disabled={!newMovie.title

|| !urlValidator(newMovie.imgUrl)

|| !urlValidator(newMovie.imdbUrl)

|| !newMovie.imdbId}

disabled={isDisabled}

trim for other required values will solve such problem as on screencast

VitaliyBondarenko1982 · 2024-01-20T13:22:21Z

src/App.tsx


 export const App = () => {
+  const [movieList, setMovieList] = useState<Movie[]>([...moviesFromServer]);


Suggested change

const [movieList, setMovieList] = useState<Movie[]>([...moviesFromServer]);

const [movieList, setMovieList] = useState<Movie[]>(moviesFromServer);

redundant to destructure

VitaliyBondarenko1982 · 2024-01-20T13:22:38Z

src/components/NewMovie/NewMovie.tsx

+  // const [description, setDescription] = useState('');
+  // const [imgUrl, setImgUrl] = useState('');
+  // const [imdbUrl, setimdbUrl] = useState('');
+  // const [imdbId, setImdbId] = useState('');


do not keep commented code

VitaliyBondarenko1982 · 2024-01-20T13:29:19Z

src/components/TextField/TextField.tsx

@@ -47,14 +50,16 @@ export const TextField: React.FC<Props> = ({
          })}
          placeholder={placeholder}
          value={value}
-          onChange={event => onChange(event.target.value)}
+          onChange={event => onChange(event)}
          onBlur={() => setTouched(true)}
        />
      </div>

      {hasError && (


trim value in hasError counting for correct show error message when fill input only with empty spaces.
const hasError = touched && required && !value.trim();

Viktor-Kost

Nice!
Some minor changes needed

Viktor-Kost · 2024-01-22T20:37:02Z

src/components/NewMovie/NewMovie.tsx

+    imdbId: '',
+  });
+
+  const urlValidator = (urlString: string): boolean => {


move this helper outside the component

Viktor-Kost · 2024-01-22T20:37:52Z

src/components/NewMovie/NewMovie.tsx

+  onAdd: (
+    movie: {
+      title: string,
+      description: string,
+      imgUrl: string,
+      imdbUrl: string,
+      imdbId: string,
+    }
+  ) => void


you have a type for a movie

Suggested change

onAdd: (

movie: {

title: string,

description: string,

imgUrl: string,

imdbUrl: string,

imdbId: string,

}

) => void

onAdd: (

movie: (movie: Movie) => void

isn't fixed

Viktor-Kost · 2024-01-22T20:38:58Z

src/components/NewMovie/NewMovie.tsx

+  const [newMovie, setNewMovie] = useState({
+    title: '',
+    description: '',
+    imgUrl: '',
+    imdbUrl: '',
+    imdbId: '',
+  });


move initial state to a variable initialMovieState (outside the component)

Suggested change

const [newMovie, setNewMovie] = useState({

title: '',

description: '',

imgUrl: '',

imdbUrl: '',

imdbId: '',

});

const [newMovie, setNewMovie] = useState<Movie>(initialMovieState);

Viktor-Kost · 2024-01-22T20:39:10Z

src/components/NewMovie/NewMovie.tsx

+    setNewMovie({
+      title: '',
+      description: '',
+      imgUrl: '',
+      imdbUrl: '',
+      imdbId: '',
+    });


Suggested change

setNewMovie({

title: '',

description: '',

imgUrl: '',

imdbUrl: '',

imdbId: '',

});

setNewMovie(initialMovie);

Viktor-Kost

Almost done!
Yet you've missed this comment
https://github.com/mate-academy/react_movies-list-add-form/pull/2120/files#r1462388794

Viktor-Kost · 2024-01-22T21:11:17Z

src/components/NewMovie/NewMovie.tsx

+  onAdd: (
+    movie: {
+      title: string,
+      description: string,
+      imgUrl: string,
+      imdbUrl: string,
+      imdbId: string,
+    }
+  ) => void


isn't fixed

etojeDenys

lgtm

add task react_movies-list-add-form

a96b11a

etojeDenys suggested changes Jan 20, 2024

View reviewed changes

combine all states into one and create handleChange

bf6902e

P-Nazar requested a review from etojeDenys January 20, 2024 12:55

VitaliyBondarenko1982 suggested changes Jan 20, 2024

View reviewed changes

fix task react_movies-list-add-form

042d002

P-Nazar requested a review from VitaliyBondarenko1982 January 22, 2024 20:29

Viktor-Kost suggested changes Jan 22, 2024

View reviewed changes

error correction task react_movies-list-add-form

726de92

P-Nazar requested a review from Viktor-Kost January 22, 2024 21:05

Viktor-Kost suggested changes Jan 22, 2024

View reviewed changes

fixed all the errors

b960dda

P-Nazar requested a review from Viktor-Kost January 23, 2024 19:39

etojeDenys approved these changes Jan 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add task react_movies-list-add-form #2120

add task react_movies-list-add-form #2120

P-Nazar commented Jan 20, 2024

etojeDenys Jan 20, 2024

VitaliyBondarenko1982 left a comment

VitaliyBondarenko1982 Jan 20, 2024

VitaliyBondarenko1982 Jan 20, 2024

VitaliyBondarenko1982 Jan 20, 2024

VitaliyBondarenko1982 Jan 20, 2024

VitaliyBondarenko1982 Jan 20, 2024

VitaliyBondarenko1982 Jan 20, 2024

Viktor-Kost left a comment

Viktor-Kost Jan 22, 2024

Viktor-Kost Jan 22, 2024 •

edited

Loading

Viktor-Kost Jan 22, 2024

Viktor-Kost Jan 22, 2024

Viktor-Kost Jan 22, 2024

Viktor-Kost left a comment

Viktor-Kost Jan 22, 2024

etojeDenys left a comment


		export const App = () => {
		const [movieList, setMovieList] = useState<Movie[]>([...moviesFromServer]);

add task react_movies-list-add-form #2120

Are you sure you want to change the base?

add task react_movies-list-add-form #2120

Conversation

P-Nazar commented Jan 20, 2024

Choose a reason for hiding this comment

VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-Kost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-Kost Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-Kost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etojeDenys left a comment

Choose a reason for hiding this comment

Viktor-Kost Jan 22, 2024 •

edited

Loading