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

feat: add validation to editableField #1425

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

evanyang1
Copy link
Member

@evanyang1 evanyang1 commented Jul 15, 2023

Fixes #1367 #1368 #1369 #1370 #1363 #1402

What changes did you make and why did you make them ?

  • Added validation to editableField. Throws alert if the changes are rejected.
  • Added validation to description.
  • Upon onBlur(), the field value is trimmed and updated as trimmed.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

image

Visuals after changes are applied

image

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b evanyang1-evan-form-validation development
git pull https://github.com/evanyang1/VRMS.git evan-form-validation

@trillium trillium changed the title add validation to editableField feat: add validation to editableField Jul 15, 2023
@trillium
Copy link
Member

Hey Evan,

Can you add a bit of context on the changes you've made to the codebase and why they've been made?

I know that we have the issue markes, and that you've referenced them within this PR, but it's good practice to put a summary together so that whoever is reviewing it has an easier time following along with the changes you've submitted.

Copy link
Member

Choose a reason for hiding this comment

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

We're planning on dropping the edits to editableField.js and keeping the utils validator

@trillium
Copy link
Member

trillium commented Aug 1, 2023

@evanyang1
We're planning on merging #1401 first which will generate merge conflits. Pause developing on this until that is merged so we can handle easier.

Copy link
Member

@trillium trillium left a comment

Choose a reason for hiding this comment

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

Thanks for the validation functions Evan!

@trillium trillium merged commit d6a9340 into hackforla:development Sep 7, 2023
4 checks passed
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.

Form Validation - Github URL
2 participants