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

added logic for add form #1804

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

added logic for add form #1804

wants to merge 5 commits into from

Conversation

irahos
Copy link

@irahos irahos commented Sep 7, 2023

Copy link

@roman-mirzoian roman-mirzoian left a comment

Choose a reason for hiding this comment

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

Good job, but there is quite a bit of work to be done.
Now you can add a movie with empty information by putting empty spaces in each input:
https://gyazo.com/1e46cc4e36099e2b7fbe36b596d76d4e

Comment on lines 8 to 12
const [title, setTitle] = useState('');
const [description, setDescription] = useState('');
const [imgUrl, setImgUrl] = useState('');
const [imdbUrl, setImdbUrl] = useState('');
const [imdbId, setImdbId] = useState('');

Choose a reason for hiding this comment

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

All this should be kept in one state.

Choose a reason for hiding this comment

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

All this should be kept in one state.

You still have a state for each input, let's create one state for all of them. and one handler to update the state when onChange event is triggered.

Your default state can looks like:

const deafultFormState = {
  title: '',
  description: '',
  imgUrl: '',
  imdbUrl: '',
  imdbId: '',
}

setImdbId('');
};

const isDisabled = () => {

Choose a reason for hiding this comment

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

Suggested change
const isDisabled = () => {
const isAddButtonDisabled = () => {

return !(title && imgUrl && imdbUrl && imdbId);
};

const submit = (): void => {

Choose a reason for hiding this comment

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

Suggested change
const submit = (): void => {
const handleSubmit = (): void => {

Copy link

@BudnikOleksii BudnikOleksii left a comment

Choose a reason for hiding this comment

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

Good work 👍
But one moment still not fixed, check the comment

Comment on lines 8 to 12
const [title, setTitle] = useState('');
const [description, setDescription] = useState('');
const [imgUrl, setImgUrl] = useState('');
const [imdbUrl, setImdbUrl] = useState('');
const [imdbId, setImdbId] = useState('');

Choose a reason for hiding this comment

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

All this should be kept in one state.

You still have a state for each input, let's create one state for all of them. and one handler to update the state when onChange event is triggered.

Your default state can looks like:

const deafultFormState = {
  title: '',
  description: '',
  imgUrl: '',
  imdbUrl: '',
  imdbId: '',
}

Copy link

@roman-mirzoian roman-mirzoian left a comment

Choose a reason for hiding this comment

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

Nice work, but be careful with the naming.

const [count, setCount] = useState(0);
const [formValues, setFormValues] = useState(defaultFormValues);

const formReset = () => {

Choose a reason for hiding this comment

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

The function name must begin with a verb.

Suggested change
const formReset = () => {
const resetForm = () => {

setCount((prevCount) => prevCount + 1);
};

const handleChange = (name: string, newValue: string) => {

Choose a reason for hiding this comment

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

The name should clearly describe what is happening in the handler:

Suggested change
const handleChange = (name: string, newValue: string) => {
const handleInputChange = (name: string, newValue: string) => {

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.

3 participants