solution react_movies_list_add_form task #1909

ivaZaiets · 2023-10-19T19:01:00Z

HardCodWorker

AWESOME!

polosanya

Almost perfect!

polosanya · 2023-10-20T15:14:38Z

src/components/NewMovie/NewMovie.tsx

+  const handleTitleUpdate = (newValue: string) => {
+    setTitle(newValue);
+  };
+
+  const handleDescriptionUpdate = (newValue: string) => {
+    setDescription(newValue);
+  };
+
+  const handleimgUrlUpdate = (newValue: string) => {
+    setimgUrl(newValue);
+  };
+
+  const handleImdbUrlUpdate = (newValue: string) => {
+    setimdbUrl(newValue);
+  };
+
+  const handleImdbIdUpdate = (newValue: string) => {
+    setImdbId(newValue);
+  };


These functions are actually redundant

polosanya · 2023-10-20T15:15:15Z

src/components/NewMovie/NewMovie.tsx

-        value=""
-        onChange={() => {}}
+        value={title}
+        onChange={handleTitleUpdate}


Suggested change

onChange={handleTitleUpdate}

onChange={setTitle}

Can be simplified just like this

polosanya · 2023-10-20T15:16:01Z

src/components/NewMovie/NewMovie.tsx

-          >
-            Add
-          </button>
+          {!title || !imgUrl || !imdbUrl || !imdbId ? (


Create a variable for this condition

polosanya · 2023-10-20T15:16:24Z

src/components/NewMovie/NewMovie.tsx

+              type="submit"
+              data-cy="submit-button"
+              className="button is-link"
+              disabled


Suggested change

disabled

disabled={condition}

And just use it here. No need to render two different buttons

StasSokolov1

Great job, let's improve your code a bit more!

StasSokolov1 · 2023-10-24T07:24:43Z

src/components/NewMovie/NewMovie.tsx

+
+  /* eslint-disable max-len */
+
+  const pattern = /^((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=+$,\w]+@)?[A-Za-z0-9.-]+|(?:www\.|[-;:&=+$,\w]+@)[A-Za-z0-9.-]+)((?:\/[+~%/.\w-_]*)?\??(?:[-+=&;%@,.\w_]*)#?(?:[,.!/\\\w]*))?)$/;


will be better move this into separated file like patterns

StasSokolov1 · 2023-10-24T07:25:35Z

src/components/NewMovie/NewMovie.tsx

+        incorrectimgUrl=""
+        incorrectimdbUrl=""


let's remove these props with empty string

StasSokolov1 · 2023-10-24T07:25:48Z

src/components/TextField/TextField.tsx

+  incorrectimgUrl,
+  incorrectimdbUrl,


Suggested change

incorrectimgUrl,

incorrectimdbUrl,

incorrectimgUrl = '',

incorrectimdbUrl = '',

StasSokolov1 · 2023-10-24T07:26:16Z

src/components/TextField/TextField.tsx

 }) => {
  // generage a unique id once on component load
  const [id] = useState(() => `${name}-${getRandomDigits()}`);

  // To show errors only if the field was touched (onBlur)
+  // const pattern = /^((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=+$,\w]+@)?[A-Za-z0-9.-]+|(?:www\.|[-;:&=+$,\w]+@)[A-Za-z0-9.-]+)((?:\/[+~%/.\w-_]*)?\??(?:[-+=&;%@,.\w_]*)#?(?:[,.!/\\\w]*))?)$/;


remove this comment

StasSokolov1 · 2023-10-24T07:26:40Z

src/components/TextField/TextField.tsx

+      {hasError ? (
        <p className="help is-danger">{`${label} is required`}</p>
+      ) : (
+        null
+      )}


Suggested change

{hasError ? (

<p className="help is-danger">{`${label} is required`}</p>

) : (

null

)}

{hasError && (

<p className="help is-danger">{`${label} is required`}</p>

)}

can be simplify

polosanya

Well done! 🔥

polosanya · 2023-10-24T14:47:11Z

src/components/NewMovie/NewMovie.tsx

+        onChange={(newValue) => {
+          setTitle(newValue);
+        }}


Suggested change

onChange={(newValue) => {

setTitle(newValue);

}}

onChange={setTitle}

Can be simplified

polosanya · 2023-10-24T14:49:11Z

src/components/NewMovie/NewMovie.tsx

+      setIncorrectimgUrl('Error');
+      setIncorrectimdbUrl('Error');


The name of the state is a bit confusing. Also, you could set the full error's text here and use it to show the user instead of the "Label is not valid"

solution react_movies_list_add_form task

771279f

HardCodWorker approved these changes Oct 19, 2023

View reviewed changes

polosanya suggested changes Oct 20, 2023

View reviewed changes

fixed mistakes + added optional advanced validation

3c90f96

ivaZaiets requested a review from polosanya October 23, 2023 15:36

StasSokolov1 suggested changes Oct 24, 2023

View reviewed changes

fixed mistakes

10709df

ivaZaiets requested a review from StasSokolov1 October 24, 2023 09:24

polosanya approved these changes Oct 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

solution react_movies_list_add_form task #1909

solution react_movies_list_add_form task #1909

ivaZaiets commented Oct 19, 2023

HardCodWorker left a comment

polosanya left a comment

polosanya Oct 20, 2023

polosanya Oct 20, 2023

polosanya Oct 20, 2023

polosanya Oct 20, 2023

StasSokolov1 left a comment

StasSokolov1 Oct 24, 2023

StasSokolov1 Oct 24, 2023

StasSokolov1 Oct 24, 2023

StasSokolov1 Oct 24, 2023

StasSokolov1 Oct 24, 2023

polosanya left a comment

polosanya Oct 24, 2023

polosanya Oct 24, 2023


		/* eslint-disable max-len */

		const pattern = /^((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=+$,\w]+@)?[A-Za-z0-9.-]+\|(?:www\.\|[-;:&=+$,\w]+@)[A-Za-z0-9.-]+)((?:\/[+~%/.\w-_])?\??(?:[-+=&;%@,.\w_])#?(?:[,.!/\\\w]*))?)$/;

solution react_movies_list_add_form task #1909

Are you sure you want to change the base?

solution react_movies_list_add_form task #1909

Conversation

ivaZaiets commented Oct 19, 2023

HardCodWorker left a comment

Choose a reason for hiding this comment

polosanya 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

StasSokolov1 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

polosanya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment