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

Remove JQuery from form validation code #160

Merged
merged 8 commits into from
Jun 23, 2021

Conversation

TimJentzsch
Copy link
Contributor

Relevant issue: N/A

Description:

This removes the JQuery used in the form validation code in #159. The functionality should be identical, to the best of my knowledge.

I also removed a potential attack vector for script injection and handling in case that the forms don't have the expected structure.

If we are to proceed with this, first move this PR and then #159.

Checklist:

  • Code Quality
  • Pep-8
  • Tests (if applicable)
  • Success Criteria Met
  • Inline Documentation
  • Wiki Documentation (if applicable)

@TimJentzsch TimJentzsch requested a review from a team as a code owner June 18, 2021 17:54
Copy link
Member

@TheLonelyGhost TheLonelyGhost left a comment

Choose a reason for hiding this comment

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

Still going through this, but some immediate input we can get to discussing

blossom/templates/website/generic_form.html Show resolved Hide resolved
blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
@itsthejoker
Copy link
Member

itsthejoker commented Jun 21, 2021

If I submit an empty form, all of the errors show up, but it also nukes all of the form input labels:

image

Expected behavior from #159

image

Copy link
Member

@itsthejoker itsthejoker left a comment

Choose a reason for hiding this comment

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

Breaks on error rendering -- see above screenshots for details

@TimJentzsch
Copy link
Contributor Author

Breaks on error rendering -- see above screenshots for details

Good catch -- should be fixed now!

Copy link
Member

@itsthejoker itsthejoker left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work!!

@itsthejoker itsthejoker merged commit 41443cd into bootstrap5 Jun 23, 2021
@itsthejoker itsthejoker deleted the bootstrap5-replace-jquery branch June 23, 2021 15:39
itsthejoker added a commit that referenced this pull request Jun 25, 2021
* convert to bootstrap5

* add support for directing back to engineering when on engineering post

* fix support for blog on mobile

* add test form

* Remove JQuery from form validation code (#160)

* Replace JQuery in input file and error message code

* Replace JQuery in form validation script

* Fix event listeners and clean up code

* Remove JQuery script

* Use query selectors for easier iteration

* Make error list clearing more efficient

* Use template string literals instead of concatenation

Co-authored-by: David Alexander <TheLonelyGhost@users.noreply.github.com>

* Fix form labels being deleted when displaying form errors

Co-authored-by: David Alexander <TheLonelyGhost@users.noreply.github.com>

* remove test form

Co-authored-by: Joe Kaufeld <joe@kenzie.academy>
Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de>
Co-authored-by: David Alexander <TheLonelyGhost@users.noreply.github.com>
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.

None yet

3 participants