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(v3.6.0): tup 230 make form plugin work #500

Merged
merged 27 commits into from
Jun 21, 2022

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Jun 14, 2022

Overview

Form plugin: update, configure, style*, document.

* The styles only use inspiration from new TUP login form. Design will later give standard CMS form styles (and later CMS-specific button styles).

Related

Changes

  • pin core-styles to version compatible with CMS v3.6.0
  • install django-recaptcha
  • replace djangocms-forms with djangocms-forms-maintained
  • isolate and document form plugin settings
  • copy c-button from core-styles v0.5+
  • (minimally) overwrite c-button
  • style form elements (generic and plugin)
  • load jQuery with AJAX (to support form)
  • sample code to customize captcha
  • override form template to:
    1. load css
    2. fix console error
  • add required url entry
  • add frontera colors for primary button

Testing

Deployed on Frontera Pre-Prod.
Tests on TUP-230 test page. (Must log in.)

  1. Create form plugin instance.
    • Complete required settings (no e-mail).1
    • Check "Save to database".
  2. Submit form—see success message.2
  3. Submit form again—see success message.2
  4. Edit form plugin instance.
    • Add SPAM PROTECTION3
  5. Submit form—see success message.2
  6. In "Site administration", open "Form submissions".
  7. Confirm your submissions are there.

Screenshots

Screenshots of Fields & Errors
Fields Errors
fields errors
Videos of Form Submission with & w/out Captcha
no.captcha.mov
with.captcha.mov

Notes

Authorized developer should add/update a Recaptcha account with domain/s and add secret key to frontera server.

  1. Confluence: Google reCAPTCHA, Analytics & Site Verification
  2. TACC/Core-Portal-Deployments Frontera CMS captcha settings

Footnotes

  1. E-mail feature is delayed until I have bandwidth to research it and configure or fix it.

  2. The success message is top center of screen white on black (a .cms_message). These should be styled better. 2 3

  3. There should only be options "None" and "Recaptcha", chose "Recaptcha"

The django-forms(-maintained) plugin expects jquery with ajax features.

Our CMS was using a slim build of jQuery.

The size difference is 17KB, which I consider negligible.
- less changes form original
- sometimes the original html was better for styling
Pointer file mimics what I do for blog CSS, which is also in source.
@wesleyboar wesleyboar changed the title V3.6.0/tup 230 make form plugin work feat(v3.6.0): tup 230 make form plugin work Jun 14, 2022
@wesleyboar wesleyboar changed the base branch from main to release/v3.6.0 June 14, 2022 20:18
Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Comments for reviewers.

@wesleyboar wesleyboar requested review from taoteg and rstijerina June 15, 2022 21:29
Copy link
Collaborator

@taoteg taoteg left a comment

Choose a reason for hiding this comment

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

Worked for me on frontera following testing steps. LGTM!

@wesleyboar
Copy link
Member Author

wesleyboar commented Jun 21, 2022

I've moved the three optional tasks that came from code review to #498.

I will merge, check pre-prod, then deploy if good (there was a UI fix since H.P. tested on pre-prod):

H.P. approved design as is (private ref.). M.S. will have new design, but I can apply it before new TACC.

@wesleyboar
Copy link
Member Author

Process I previously mentioned for deploy was edited; new step to test final UI fix on pre-prod.

@wesleyboar wesleyboar merged commit 938b9b3 into release/v3.6.0 Jun 21, 2022
@wesleyboar wesleyboar deleted the v3.6.0/tup-230-make-form-plugin-work branch June 21, 2022 13:57
wesleyboar added a commit that referenced this pull request Jul 6, 2022
Not a straight clone of #500.

I added a little CSS and a small document to avoid cloning one template.

See:
- default.html.md
- django.cms.forms.css
Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

I have migrated or improved these changes into #498.

wesleyboar added a commit that referenced this pull request Jul 15, 2022
* feat(tup-230): working form plugin

* docs(tup-230): clarify form submission failure

* feat(tup-230): use jquery version that has ajax

The django-forms(-maintained) plugin expects jquery with ajax features.

Our CMS was using a slim build of jQuery.

The size difference is 17KB, which I consider negligible.

* feat(tup-230): make spam protection available

* feat(tup-230): e-mail template (when e-mail works)

* docs(tup-230): tweak comment in settings

* fix(tup-230): comments & semantic html in template

* fix(tup-230): revert and make some html changes

- less changes form original
- sometimes the original html was better for styling

* feat(tup-230): style form (no design review yet)

Also, no button styling yet.

* chore(tup-230): rename css file, add pointer file

Pointer file mimics what I do for blog CSS, which is also in source.

* chore(settings): tup-308, how to set captcha keys

Ref: TACC/Core-Portal-Deployments@48e6e9c

* noop(settings): tup-308, add new line

* fix(settings): tup-308, use new form css from #500

* fix(settings): tup-308, new form templates by #500

Not a straight clone of #500.

I added a little CSS and a small document to avoid cloning one template.

See:
- default.html.md
- django.cms.forms.css

* noop(settings): tup-308, form settings from #501

* fix(settings): tup-308, css changes from #501

* fix(settings): tup-308, "*" to "(required)"

I use a different solution than #501.

In #501, markup was changed to "(require)":
https://github.com/TACC/Core-CMS/blob/5f63bdf/taccsite_cms/templates/djangocms_forms/form_template/default.html#L27

But here, I do not edit the markup, so no need to clone template:
https://github.com/TACC/Core-CMS/blob/8a46a21/taccsite_cms/templates/djangocms_forms/form_template/default.html.md

* fix(css): tup-308, small form style issues

- bigger gap between single checkbox and input
- add gap between multiple checkbox and input
- align multiple checkbox and input
- simpler, less specific checkbox label selector
- remove undesired label margin*

* This style was ineffectual—good—in 3.6, but "fixed"—bad—when ported.

* fix(css): tup-308, small form style issues

- limit which help text gets margin
- remove ineffectual, unnecessary margin removal
- bigger gap between single checkbox and input
- align single checkbox and input
- add gap between multiple checkbox and input
- align multiple checkbox and input
- remove label margin on last of multiple checkboxes/radios
- simpler, less specific checkbox label selector
- remove undesired label margin*

* This style was ineffectual—good—in 3.6, but "fixed"—bad—when ported.

* docs(styles): typo fix for KSS comment

* fix(form): tup-308 only working export formats

1. Only let users export with supported formats.
2. Update form version to rev. that supports this.

Source: avryhof/djangocms-forms#8

* fix(form): delete unnecessary cloned template

* fix(form): tup-308, clean up templates

- remove unedited cloned template
- update commit in a comment

* fix(form): tup-308, move form export settings

* fix(form): tup-308, remove unused file

I forgot to delete this file earlier.

* fix(form): tup-308, remove redundant script

The removal happens form a merged PR on the plugin:
avryhof/djangocms-forms#12

* chore(core-styles): v0.7.0-beta

* fix(taccsite_custom): get main relevant PRs merged

* docs(form): no commented settings
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