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

Disable frequency/interval fields if not required. Mark required if they are so they are validated before submit #17526

Merged

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jun 6, 2020

Overview

Support js validation on frequency interval field on frontend contribution page

Before

No jquery validation on frequency interval on frontend contribution page.

After

jquery validation works. Fields are disabled when not required:
ezgif com-crop

Technical Details

If the "i want to contribute this amount every" checkbox is checked then the frequency interval is a required field. But it's not marked as required so only fails validation on the php side once the form is submitted.

Comments

Related issue #16495

@civibot
Copy link

civibot bot commented Jun 6, 2020

(Standard links)

@civibot civibot bot added the master label Jun 6, 2020
@eileenmcnaughton
Copy link
Contributor

I haven't tested but this seems sensible - can we use meaningful names though - it's a lot easier to understand frequencyUnit than frUnit

@mattwire
Copy link
Contributor Author

mattwire commented Jun 8, 2020

@eileenmcnaughton There is similar code elsewhere (for backend forms) so I just copied the variables. Have updated now per suggestion

@eileenmcnaughton
Copy link
Contributor

@jaapjansma if you are still going this is good IMHO from a code & concept POV - if you get a chance to -run I'm happy to merge this one (or for Matt to do so based on a thumbs up from you or anyone else who r-runs it)

@jaapjansma
Copy link
Contributor

@mattwire this PR has conflicting files. Could you please fix this?

@jaapjansma jaapjansma added needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state and removed ready for review labels Jun 29, 2020
@mattwire mattwire force-pushed the frontendrequiredpaymentfrequency branch from 3e3722b to 14ab579 Compare June 30, 2020 13:11
@mattwire mattwire force-pushed the frontendrequiredpaymentfrequency branch from 14ab579 to 2a95981 Compare June 30, 2020 13:12
@mattwire
Copy link
Contributor Author

@jaapjansma Thankyou - I've rebased

@mattwire mattwire added ready for review and removed needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state labels Jun 30, 2020
@eileenmcnaughton
Copy link
Contributor

I tested this & was able to replicate it - it was pretty confusing because there IS validation that shows regardless - it's just some of it is jquery (& hence faster)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants