-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
add task solution #2088
base: master
Are you sure you want to change the base?
add task solution #2088
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done!👍
|
||
const sumbitCheck = !( | ||
title.trim() && imageUrl.trim() && imdbUrl.trim() && imdbId.trim() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, forgot to redeploy
src/components/NewMovie/NewMovie.tsx
Outdated
const [description, setDescription] = useState(''); | ||
const [imageUrl, setImageUrl] = useState(''); | ||
const [imdbUrl, setImdbUrl] = useState(''); | ||
const [imdbId, setImdbId] = useState(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of creating a lot of states, use just one
const defaultMovie = {
title: '',
description: '',
imgUrl: '',
imdbUrl: '',
imdbId: '',
};
const [count, setCount] = useState(0);
const [newMovie, setNewMovie] = useState(defaultMovie);
src/App.tsx
Outdated
</div> | ||
<div className="sidebar"> | ||
<NewMovie /* onAdd={(movie) => {}} */ /> | ||
<NewMovie onAdd={movie => setMovies([...movies, movie])} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this function into separate handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! All good!
Check submit validation. Made it more readable.
Thanks!
onChange={(newValue => setForm({ | ||
...form, | ||
imdbId: newValue, | ||
}))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write a handler for it ( not critical )
onChange={(newValue => setForm({ | ||
...form, | ||
imdbUrl: newValue, | ||
}))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write a handler for it ( not critical )
onChange={(newValue => setForm({ | ||
...form, | ||
imgUrl: newValue, | ||
}))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write a handler for it ( not critical )
onChange={(newValue => setForm({ | ||
...form, | ||
description: newValue, | ||
}))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write a handler for it ( not critical )
onChange={(newValue => setForm({ | ||
...form, | ||
title: newValue, | ||
}))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write a handler for it ( not critical )
src/components/NewMovie/NewMovie.tsx
Outdated
type AddMovie = { | ||
onAdd: (movie: Movie) => void | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can move it in another file ( not critical )
src/components/NewMovie/NewMovie.tsx
Outdated
const sumbitCheck = !( | ||
form.title.trim() && form.imgUrl.trim() | ||
&& form.imdbUrl.trim() && form.imdbId.trim() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to rewrite for best readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 🔥
Let's make some improvements 🧑💻
src/App.tsx
Outdated
|
||
export const App = () => { | ||
const [movies, setMovies] = useState([...moviesFromServer]); | ||
|
||
const handleMovie = (movie: Movie) => setMovies([...movies, movie]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use functional setState
const handleMovie = (movie: Movie) => setMovies([...movies, movie]); | |
const handleMovie = (movie: Movie) => setMovies((...) => ...); |
src/components/NewMovie/NewMovie.tsx
Outdated
|
||
export const NewMovie = () => { | ||
type AddMovie = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use interface for Props
type AddMovie = { | |
interface Props = { |
src/components/NewMovie/NewMovie.tsx
Outdated
const [form, setForm] = useState({ | ||
title: '', | ||
description: '', | ||
imgUrl: '', | ||
imdbUrl: '', | ||
imdbId: '', | ||
} as Movie); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [form, setForm] = useState({ | |
title: '', | |
description: '', | |
imgUrl: '', | |
imdbUrl: '', | |
imdbId: '', | |
} as Movie); | |
const [form, setForm] = useState<Movie>({ | |
title: '', | |
description: '', | |
imgUrl: '', | |
imdbUrl: '', | |
imdbId: '', | |
}); |
src/components/NewMovie/NewMovie.tsx
Outdated
|
||
const handleSubmit = (event: React.FormEvent) => { | ||
event.preventDefault(); | ||
setCount(count + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functional setState
src/components/NewMovie/NewMovie.tsx
Outdated
setForm({ | ||
title: '', | ||
description: '', | ||
imgUrl: '', | ||
imdbUrl: '', | ||
imdbId: '', | ||
}); | ||
|
||
onAdd(form); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first, you send the form, after - do reset
setForm({ | |
title: '', | |
description: '', | |
imgUrl: '', | |
imdbUrl: '', | |
imdbId: '', | |
}); | |
onAdd(form); | |
onAdd(form); | |
setForm({ | |
title: '', | |
description: '', | |
imgUrl: '', | |
imdbUrl: '', | |
imdbId: '', | |
}); |
value="" | ||
onChange={() => {}} | ||
value={form.title} | ||
onChange={(newValue => setForm({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont understand how to do it(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great.
Approved!
Many thanks.
DEMO LINK