UI update: regex checking for Project Name/Tag inputs in edit metadata modal #319

sanghoonio · 2024-06-17T23:31:22Z

Inputs for Project Name and Tag will only allow alphanumeric characters, '_', and '-'. If a user attempts to type an invalid character or paste a string with an invalid character into these inputs, the input will not let them and some helper text will appear to let the user know which characters are supported.

Someone can work around this by going into browser inspect element or call the API endpoint itself with invalid characters, so we should still have some API level check for these strings.

updated hands on table

Integrate views into the user interface

Critical bug fixes

…nd tag for metadata modal. added conditional helper text that appears if user enters invalid character.

nleroy917

Nice start. I totally agree with introducing another state variable to verify that the name and tag are correct. A couple thoughts and ideas...

Instead of using onKeyDown and onPaste callbacks, could we tie the check directly to the onChange callback? It gets messy with react-hook-form? Here is an idea:

We are already "watching" the new name and the new tag. Could we add an effect? Something like:

useEffect(() => {
   checkName(newName)
   checktag(newTag)
}, [newName, newTag])

Does that make sense? We could go over it together if needed. Lot of React quirks here.

As I think you rightly pointed out, this needed to be validated on the API too. Do you feel comfortable tackling that too? Hows your Python? Moreover, we need to run this validation when someone submits a new PEP too.

I believe there a lot in the src/components/forms/ folder that need this logic too.

Let me know if that all made sense!

Here is some reading on uesEffect. With great power, though, comes great responsibility. useEffect is often called useFootGun and I try to use it as little as possible. I really like this article by Dan Abromov, one of the creators of React.

web/src/components/forms/edit-project-meta.tsx

sanghoonio · 2024-06-18T16:55:57Z

Changes addressed:

onKeyDown and onPaste removed for useEffect. Users can now input or paste invalid characters into the input field, which will trigger helper text to display due to change in state variable. If the invalid characters are removed from the inputs, the state variable will revert to false and helper text will clear.
Modal save button now tracks state variable and is deactivated if name or tag contain an invalid character.
Use some bootstrap classes for the helper text. Bootstrap by default goes down to fs-6 minimum which is 16px or 1em. It looked too big to me so I switched it from 12px to 0.75em, which should help it scale if needed.

Thanks for the pointers on useEffect(). It was pretty confusing at first because it tracks the input value itself and not the key down or paste event but it seems to work pretty well.

nleroy917

🤦🏻‍♂️ I love how I sent a link to an article telling you not to use useEffect, then proceeded to say lets use useEffect.

I think I was totally wrong now that I am looking at it. Instead of the effect, I think we should just define the variable:

export const ProjectMetaEditForm = () => {

  const nameIsBad =  /[^0-9a-zA-Z_-]/.test(newName);
  const tagIsBad = /[^0-9a-zA-Z_-]/.test(newTag);
  
  return (
    <div>
    { nameIsBad ? <p className = 'text-secondary pt-1' style = {{fontSize: '.75em'}}>Invalid name. Project names should only contain letters, numbers, etc etc etc </p> : null }
    </div>
  )
}

Because useState triggers a rerender anyways, there's no need to tie all the to a useEffect, so that's my bad for giving bad advice there.

Finally... do you think you could implement something similar on the other components?

nleroy917 · 2024-06-19T12:08:42Z

web/src/components/forms/edit-project-meta.tsx

@@ -252,7 +248,7 @@ export const ProjectMetaEditForm: FC<Props> = ({
        <button
          onClick={() => mutation.mutate()}
          id="metadata-save-btn"
-          disabled={(!isDirty && isValid) || mutation.isPending}
+          disabled={(!isDirty && isValid || badName || badTag) || mutation.isPending}


Great! This is perfect.

…other components and updated error message contents and formatting. added mode: 'onChange' for formState instantation for these 3 other components.

sanghoonio · 2024-06-20T16:05:18Z

Done, updated checks in other components

nleroy917

This is great -- I love that you found those react-hook-form APIs and used them. Thats a great solution. Could we emulate that over on the edit-project-meta form? I think thats the only one using the old system.

nleroy917 · 2024-06-20T19:46:25Z

web/src/components/forms/edit-project-meta.tsx

@@ -119,6 +119,9 @@ export const ProjectMetaEditForm: FC<Props> = ({
    }
  }, [newPop]);

+  const isBadName = newName ? (/[^0-9a-zA-Z_-]/.test(newName) ? "Project Name must contain only alphanumeric characters, '-', or '_'." : null) : "Project Name must not be empty."
+  const isBadTag = newTag ? (/[^0-9a-zA-Z_-]/.test(newTag) ? "Project Tag must contain only alphanumeric characters, '-', or '_'." : null) : "Project Tag must not be empty."


Actually, the project tag can be empty.

I really loved what you did in the blank-project-form component, using build in react-hook-form API's to validate -- should we do that here?

Actually, the project tag can be empty.

I tried submitting a blank tag edit on the deployed UI and it doesn't let you so I assumed we weren't supposed to allow it. Is it supposed to default to 'default' if empty?

nleroy917 · 2024-06-21T17:17:27Z

web/src/components/forms/blank-project-form.tsx

+              value: /^[a-zA-Z0-9_-]+$/,
+              message: "Project Name must contain only alphanumeric characters, '-', or '_'.",


This is awesome, I didn't know this was possible with react-hook-form!

// instantiate form const { reset: resetForm, register, control, watch, setValue, formState: { isValid, errors }, } = useForm<FromFileInputs>({ mode: 'onChange', <<<---------- had to add this for it to work defaultValues: { is_private: false, pep_schema: 'pep/2.1.0', namespace: defaultNamespace || user?.login || '', }, });

So I added that mode: 'onChange' line to make it work but wasn't sure if it would cause lag if it started checking after every single input. Is this negligible with react/our use case or is it something we have to worry about?

nleroy917 · 2024-06-21T17:17:55Z

web/src/components/forms/blank-project-form.tsx

+      <ErrorMessage errors={errors} name="project_name" render={({ message }) => message ? (<p className='text-secondary pt-1' style={{fontSize: '.75em'}}>{message}</p>) : null} />
+      <ErrorMessage errors={errors} name="tag" render={({ message }) => message ? (<p className='text-secondary pt-1' style={{fontSize: '.75em'}}>{message}</p>) : null} />


Feel like I've tried to use this before with mixed results. Have you tried?

Just tried it myself and it looks great! What do you think about making the error message text red? (danger)

web/src/components/forms/pop-form.tsx

web/src/components/forms/project-upload-form.tsx

…ondary to text-danger

sanghoonio

Changes reviewed

…r message for when both name and tag are bad for all components. added sample count to project page. cleaned up project page slightly (borders, padding, etc).

nleroy917

Looks great! We should probably change the base to merge into dev instead of master

khoroshevskyi and others added 5 commits February 8, 2024 23:10

Merge pull request #296 from pepkit/dev

3980978

updated hands on table

Merge pull request #299 from pepkit/dev

7a6a64c

Integrate views into the user interface

Merge pull request #302 from pepkit/dev

367ae24

Critical bug fixes

added regex checking (alphanumeric, _, -) for updating project name a…

ea0fc47

…nd tag for metadata modal. added conditional helper text that appears if user enters invalid character.

added check project name and tag on paste

0acf9b2

nleroy917 requested changes Jun 18, 2024

View reviewed changes

sanghoonio added 2 commits June 18, 2024 12:56

switch to useEffect()

cfb32cd

add case for when name or tag are empty

ac4a07f

nleroy917 requested changes Jun 19, 2024

View reviewed changes

removed useEffect() and set variable instead. updated regex check in …

1fbfb82

…other components and updated error message contents and formatting. added mode: 'onChange' for formState instantation for these 3 other components.

remove unused import

ad1521a

nleroy917 requested changes Jun 21, 2024

View reviewed changes

nleroy917 and others added 2 commits June 21, 2024 13:24

remove some unused imports

91bef9c

use react-hook-form for edit-project-meta components. update text-sec…

6c8fcee

…ondary to text-danger

sanghoonio commented Jun 25, 2024

View reviewed changes

added name and tag error messages for fork modal. added combined erro…

23b0791

…r message for when both name and tag are bad for all components. added sample count to project page. cleaned up project page slightly (borders, padding, etc).

sanghoonio mentioned this pull request Jun 26, 2024

[UI] add sample count on the project page #320

Closed

updated combined name, tag error messages

2983f32

nleroy917 approved these changes Jun 26, 2024

View reviewed changes

khoroshevskyi changed the base branch from master to dev June 26, 2024 15:08

sanghoonio added 3 commits June 26, 2024 13:14

project component ui

0e1d43b

minor height adjustment

a4f2032

color adjustments

b93909f

sanghoonio merged commit 5811ce4 into dev Jun 26, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI update: regex checking for Project Name/Tag inputs in edit metadata modal #319

UI update: regex checking for Project Name/Tag inputs in edit metadata modal #319

sanghoonio commented Jun 17, 2024

nleroy917 left a comment •

edited

Loading

sanghoonio commented Jun 18, 2024 •

edited

Loading

nleroy917 left a comment

nleroy917 Jun 19, 2024

sanghoonio commented Jun 20, 2024

nleroy917 left a comment

nleroy917 Jun 20, 2024

nleroy917 Jun 21, 2024

sanghoonio Jun 21, 2024

nleroy917 Jun 21, 2024

sanghoonio Jun 21, 2024

nleroy917 Jun 21, 2024

nleroy917 Jun 21, 2024

sanghoonio left a comment

nleroy917 left a comment

		value: /^[a-zA-Z0-9_-]+$/,
		message: "Project Name must contain only alphanumeric characters, '-', or '_'.",

		<ErrorMessage errors={errors} name="project_name" render={({ message }) => message ? (<p className='text-secondary pt-1' style={{fontSize: '.75em'}}>{message}</p>) : null} />
		<ErrorMessage errors={errors} name="tag" render={({ message }) => message ? (<p className='text-secondary pt-1' style={{fontSize: '.75em'}}>{message}</p>) : null} />

UI update: regex checking for Project Name/Tag inputs in edit metadata modal #319

UI update: regex checking for Project Name/Tag inputs in edit metadata modal #319

Conversation

sanghoonio commented Jun 17, 2024

nleroy917 left a comment • edited Loading

Choose a reason for hiding this comment

sanghoonio commented Jun 18, 2024 • edited Loading

nleroy917 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanghoonio commented Jun 20, 2024

nleroy917 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

Choose a reason for hiding this comment

sanghoonio left a comment

Choose a reason for hiding this comment

nleroy917 left a comment

Choose a reason for hiding this comment

nleroy917 left a comment •

edited

Loading

sanghoonio commented Jun 18, 2024 •

edited

Loading