Disable frequency/interval fields if not required on backend contribution forms #17889
Conversation
|
(Standard links)
|
mattwire
force-pushed
the
freezerecur
branch
2 times, most recently
from
812f8d1 to
d29c263
Compare
mattwire
changed the title
mattwire
changed the title
|
@mattwire the principle for the main part seems fine & it just needs an r-run However, I don't have a framework for processing the tpl changes. Obviously the nbsp were put there for a reason - maybe a bad one - but a reason. If we are going to start stripping them out I think we need to have at least some form of discussion around that - probably in gitlab. |
|
@eileenmcnaughton not that I know of. The worst consequence I think would be someone with a custom theme having worse padding / a minor overlap - but if it looks ok with Grenwich, I don't think there's a goal to support custom themes / overrides. That said, this PR is a good reminder that the |
|
@agh1 In the case of |
|
@mattwire – I used https://fontawesome.com/icons/info-circle in Finsbury, but could have used https://fontawesome.com/icons/info. Do you reckon UI framework docs is the best place to document recommended key icons for consistency? |
Yes, it would be good to replace this https://docs.civicrm.org/dev/en/latest/framework/ui/#older-icon-system with a mapping from old to new. |
|
OK so for this to get merged I guess we need an r-run on the actual issue and someone, not necessarily the same person, to OK the tpl changes |
|
Digging a bit deeper into non-breaking spaces, I realise that their designed use is not for fixed/guaranteed spacing as I suggested above, but to keep words together when wrapping (e.g. So the question here more I guess is: is there anything in the tpl that if it now wraps where it wouldn't before, that could cause a problem? E.g. line 262: ( |
|
test this please |
|
Shall we split this in 2? I'm OK to review the js part but I can't review the other part |
|
@eileenmcnaughton I've removed the |
|
@monishdeb could you do a review including a r-run? Thx. |
|
test this please |
|
I tested this & the disabling / enabling was nice and clear. The validation took a bit more work to test. I enabled the (hidden) core processor for Eway and it turned out to be pretty bad both before and after. If I checked the recur box and then switched to eway the box was hidden. I got a processor error without this patch and after I got a jquery validation error but the checkbox was hidden so it was unclear how to resolve it. On this issue the patch is no worse than without the patch so disregarding that I think it's mergable based on the disable part working & the code seeming sensible |
|
Thanks @eileenmcnaughton - agree the form is still a bit broken / buggy and I may look at that when I get chance. But that can build on top of this :-) |
|
cool - I guess the idea would be to unset the recurring check box when it's hidden but it's probably not a big real world issue |
Overview
Follow up to #17526
Freeze the recur frequency interval/unit fields if recur is not selected. This does the same thing that #17526 did for the contribution frontend form.
Support js validation on frequency interval field on backend contribution page.
Before
Fields not disabled when recurring is not selected. Recur fields sometimes do not load for the default processor when there are multiple processors on the form and the default supports recurring.
After
Disabled:
Enabled:
Fields appear for the default processor if it supports recur.
Technical Details
Comments
Seems that the fields only appear if your default payment processor has some paymentfields defined (eg. creditcard or bank details). Stripe, gocardless etc. do not... But the authorizenet extension does (as would the core version). Then just open the standard "submit credit card" form on the contribution tab and you should see options to enter "every x month/year"