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

Do not launch raw js alert jqueryValidation fails #14854

Merged
merged 1 commit into from
Jul 28, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 22, 2019

Overview

Jquery validation is not enabled much on front end forms but if you DO enable it you get very nasty pure js alert messages. This removes those - after discussion removal makes sense at this stage. You still see the red messages and it's still possible for extensions to capture & render the messages with some jquery foo.

To enable validation on contribution pages in an extension assign

$form->assign('isJsValidate', TRUE); - it will kick in if you try to submit with an invalid email

Before

Cannot enable Validate without horrendous js alerts

After

Can....

Technical Details

This isn't my ultimate preference - per long discussion here
eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor#128

I think being able to output a summary of messages near, or in place of, the paypal
checkout button makes more sense - open to better options @colemanw

Comments

@colemanw this is what I've been kinda back-ground nagging you about - I had a go - much more detail on the link above

@civibot
Copy link

civibot bot commented Jul 22, 2019

(Standard links)

@civibot civibot bot added the master label Jul 22, 2019
@mattwire
Copy link
Contributor

@eileenmcnaughton Any chance of a quick screenshot so I can be certain which alert you're talking about?

@colemanw
Copy link
Member

I don't feel quite right about this. Maybe we should just get rid of the alert() altogether. Or possibly fix the crmError function to not raise an alert. I think a more general fix for clientside validation on frontend forms is needed.

@eileenmcnaughton
Copy link
Contributor Author

@colemanw I'm happy with any of those options :-) Just removing the alert seems easiest but I feel like in the case of paypal checkout it might be good to display it close to the buttons - if you grab that pr on paypal checkout you'll see how it looks - oh & while you are at it you could dig into why it doesn't work right with webform :-)

…ages

It's really really ugly & as discussed without it
a) extensions can still opt to display the errors themselves and
b) the inline errors still appear.

Note this will have no real affect on existing code - the ugliness of this has been
a blocker to getting into front end validation & at this stage only the
omnipay extension is trying to work in this space.
eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor#128

I think ideally with Omnipay it DOES makes sense to display messages
near the checkout button - which might be a future step
@eileenmcnaughton eileenmcnaughton changed the title Allow extensions to suppress the popups for jqueryValidation Do not launch raw js alert jqueryValidation fails Jul 27, 2019
@eileenmcnaughton
Copy link
Contributor Author

@colemanw as discussed - just removing the nasty alerts now

@colemanw
Copy link
Member

Agreed. The alert doesn't add anything useful, just annoyance.

@colemanw colemanw merged commit 84fcc93 into civicrm:master Jul 28, 2019
@colemanw colemanw deleted the validate branch July 28, 2019 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants