[NFC] Preferences form template cleanup

[eileenmcnaughton]

Overview

Moves code around to a more logical place.

Before

When the invoicing code was added it was added to the preferences template...badly. Instead of adding code related to invoicing to the relevant preferences template it was added to a shared template, wrapped in 'ifs'.

After

Code specific to only one preference form moved back to it's own template & shared code simplified.

Technical Details

Just moves code around - no change. Note each commit is a distinct small step in this process

Comments

There are still a couple of things to fix on the php side on this form - but this neither creates or fixes those issues.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@mattwire almost there for the cleanup round - ping

[mattwire]

@eileenmcnaughton I don't understand why but the checkbox to show/hide tax/invoicing settings seems to have changed it's ID following this PR to #invoicing_invoicing which stopped the jQuery from working. The jQuery was also rather old in that function, I replaced it as follows:

{literal}
  <script type="text/javascript">
    CRM.$(document).ready(function() {
      if (CRM.$("#invoicing_invoicing").prop('checked')) {
        CRM.$("#invoicing_blocks").show();
      }
      else {
        CRM.$("#invoicing_blocks").hide();
      }
    });
    CRM.$(function() {
      CRM.$("input[type=checkbox]").click(function() {
        if (CRM.$("#invoicing_invoicing").prop('checked')) {
          CRM.$("#invoicing_blocks").show();
        }
        else {
          CRM.$("#invoicing_blocks").hide();
        }
      });
    });
  </script>
{/literal}

[pradpnayak]

Hi @mattwire

How about below code to hide/show invoice block

{literal}
  <script type="text/javascript">
    CRM.$(function($) {
      hideShowInvoiceBlocks();
      $("#invoicing_invoicing").click(hideShowInvoiceBlocks);

      function hideShowInvoiceBlocks() {
        if ($("#invoicing_invoicing").prop('checked')) {
          $("#invoicing_blocks").show();
        }
        else {
          $("#invoicing_blocks").hide();
        }
      }
    });
  </script>
{/literal}

[mattwire]

@pradpnayak Yes, much better and cleaner. I've just tested your version and it works perfectly.

[pradpnayak]

@JoeMurray / @Edzelopez / @monishdeb

You might need to update your extension code like EasyBatch, Edit Line Item, Close Accounting Period etc which add's some settings using jQuery and CiviCRM hooks on Contribute Settings page.

Thanks
Pradeep

[eileenmcnaughton]

@mattwire @pradpnayak can we merge this & then @pradpnayak's version as a follow up? There is one more PR based on this #13047 which isolates that form

[eileenmcnaughton]

also @pradpnayak you may not have seen this https://lab.civicrm.org/dev/core/issues/495 - if I can get the last fix-ups merged I'll work on that & it will give better options for extensions

[eileenmcnaughton]

@mattwire I think with #13047 merged the jquery works again - this neither breaks or fixes it

[eileenmcnaughton]

(but agree @pradpnayak's is an improvement)

[mattwire]

@eileenmcnaughton Ok, I see it is currently broken in master too. In which case let's merge this and resolve see if the jquery is fixed by #13047 (but still be good to update to use @pradpnayak changes afterwards).

[eileenmcnaughton]

@mattwire thanks - merging - once #13047 is merged we need to do a good audit!

[eileenmcnaughton]

& yes agree re @pradpnayak improvement - also, I don't understand WHY the id changed - but at the moment I think merge #13047 & then look at that

[pradpnayak]

Hi @eileenmcnaughton

I just tested on my local machine on current state of code. It appears that labels for field is printed twice. Is this something fixed in your other PR?

[eileenmcnaughton]

@pradpnayak let's hope so :-)

[pradpnayak]

@eileenmcnaughton I am going to test #13047 now

[eileenmcnaughton]

thanks @pradpnayak