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

Add hack to support buttonrama without change to code adding submit #307

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

This is a bit hacky but I looked at cividiscount & concluded we should support the existing
code if we could - if nothing else then to simplify messaging about buttonrama. With this change no change is required
to cividiscount (although I did put up another change to cividiscount which I found to be broken on a different issue civicrm/org.civicrm.module.cividiscount#242)

Without this

Screen Shot 2020-09-11 at 4 53 23 PM

With this

Screen Shot 2020-09-11 at 4 47 42 PM

Having figured this out I'm not 100% sure it was worth the work I put in :-). However, it does mean that no version specific code is required in extensions. I think only a small handle of extensions are affected (including on by @agh1 who I'm sure knows what to do) - but OTOH the way cividiscount does it is nasty & not having to change that right now feels like a bonus

This is a bit hacky but I looked at cividiscount & concluded we should support the existing
code if we could - if nothing else then to simplify messaging about buttonrama. With this change no change is required
to cividiscount (although I did put up another change as it turned out not to work
@civibot
Copy link

civibot bot commented Sep 11, 2020

(Standard links)

@civibot civibot bot added the master label Sep 11, 2020
@seamuslee001
Copy link
Contributor

I tested this and confirmed it works with the CiviDiscount extension and will assist with the transition merging

@seamuslee001 seamuslee001 merged commit 13af4d5 into civicrm:master Sep 30, 2020
@eileenmcnaughton
Copy link
Contributor Author

thanks @seamuslee001

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.

2 participants