dev/core#4310 Fix layout regression in MembershipBlock for Contribution pages

[agileware-fj]

Overview

Fixes a regression where the membership block escapes its containing div on contribution pages, cascading a layout error further down the page.

See dev/core#4310

Before

The membership block escapes its containment matrix, manifesting in the browser as a layout nesting issue.

After

Membership block is contained again; also the forced renewal javascript is included again.

Technical Details

Template change only.

Comments

Happy to delete the javascript change from the PR, since I'm not sure if that's actually part of the address regression.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@agileware-fj can you put this against the 5.62 rc

[eileenmcnaughton]

@agileware-fj if you can update the branch and preferably also create the same PR against 5.61 then we can probably get this out today as we are planning a point release today

The javascript just moves stuff form the looks?

[eileenmcnaughton]

@agileware-fj can you talk me through the js move - it looks to me like the js should always be available when context = 'makeContribution' - ie it is called from the page Main.php & that it should be just in that top level if

FWIW I want to split that tpl into 2 - because there is actually 2 files in one - one is what to do for the Main.php (context = makeContribution) & the other one (or 2) is the Confirm & ThankYou usage & the overlap is pretty small

[eileenmcnaughton]

Could we put the js right before this if - this if is effectively 'if this is the Main page'- so it would always be present - I have a feeling {elseif $lineItem and $priceSetID AND !$is_quick_config} is never true but it seems to be in there

[agileware-fj]

@eileenmcnaughton that {/if} is after a couple of {elseif}s, so not so much, no, as that will leave it outside of the makeContribution context and it won't do anything useful there, rendering the move pointless. This is one of the changes in the commit that introduced this regression.

Right after the {if} might make sense; however it's worth noting that there's no autorenewal checkbox on either the review or thank you pages, meaning it's pointless on those pages anyway.

[eileenmcnaughton]

@agileware-fj yep - if should only be in the context = makeContribution - & always there when it is I think?

[agileware-fj]

@agileware-fj can you talk me through the js move - it looks to me like the js should always be available when context = 'makeContribution' - ie it is called from the page Main.php & that it should be just in that top level if

Which is effectively where I've moved it to.... ( the old top level if was removed in dfc5fb9 )

Anyway, this branch is based on 5.62 now.

[eileenmcnaughton]

Test fail seems unrelated so I think I'm OK with this then

@agileware-fj my understanding is that this elseif is actually never true & we should remove in master?

{elseif $membershipBlock AND !$is_quick_config}

[eileenmcnaughton]

Or at least the contents are meaningless...

[agileware-fj]

Or at least the contents are meaningless...

That definitely looks like it's the case

[agileware-justin]

Thanks for the quick review and merging @eileenmcnaughton 👍

[eileenmcnaughton]

thanks for the patch ! @agileware-fj & @agileware-justin