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

Afform - Support embedding forms via WP shortcodes. #19687

Merged
merged 2 commits into from Apr 7, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Feb 27, 2021

Overview

This adds support for Afform shortcodes. It allows any Custom Form or Search Form to be embedded in a WP page or post.

image


image

Technical Details

There were 2 technical challenges that have been fixed for this PR:

  1. In order for the EntityRef widget to fully work, a v3 API needed to be added for Afform, and APIv3 needed improvements to the _civicrm_api3_basic_array_get function to accept more operators than just =. Fixed via APIv3 - Improve array-based apis to support sorting and operators #19690

  2. Civi's shortcode handling needed to be updated so that content could be loaded via callback instead of running a server route. Fixed via Allow shortcodes to be rendered without invoking Civi page civicrm-wordpress#244

@civibot
Copy link

civibot bot commented Feb 27, 2021

(Standard links)

@civibot civibot bot added the master label Feb 27, 2021
@christianwach
Copy link
Member

The server_route requirement could probably be lifted either by creating a generic route for afforms or removing the requirement of invoking a Civi page entirely, as that's really not necessary in the case of Angular pages, which only require Civi to be bootstrapped and Angular modules to be loaded.

@colemanw As per my message on Mattermost... the [civicrm] Shortcode doesn't have any intrinsic requirement for a CiviCRM route - they are only present in order to invoke CiviCRM and retrieve whatever markup is returned.

We can easily make an exception for Afform where the args are defined and bypass invoking CiviCRM if that suits Afform.

If Afform is independent of q routes, this opens up lots of interesting possibilities.

@colemanw
Copy link
Member Author

Per discussion with @christianwach I've fixed the initial limitations.

colemanw added a commit to colemanw/civicrm-wordpress that referenced this pull request Feb 28, 2021
This allows civi core or extensions to supply content for shortcodes without rendering a page;
they can simply implement `civicrm_shortcode_get_data` and return the content.

Previously this filter was only invoked when multiple shortcodes were present, now it is invoked
in all cases if the shortcode attributes do not provide a `q` callback path.

See civicrm/civicrm-core#19687
@eileenmcnaughton
Copy link
Contributor

Quick work @colemanw - we discussed this as a good idea 2 days AFTER you left the meeting & now magically there is a PR.

colemanw added a commit to colemanw/civicrm-wordpress that referenced this pull request Mar 1, 2021
This allows civi core or extensions to supply content for shortcodes without rendering a page;
they can simply implement `civicrm_shortcode_alter_conent` and return the content.

This new filter is called whenever displaying contents of a single shortcode and will allow
any shortcode contents to be displayed or modified.

See civicrm/civicrm-core#19687
@totten
Copy link
Member

totten commented Mar 1, 2021

If Afform is independent of q routes, this opens up lots of interesting possibilities.

Yes, I think this should be done independent of q/routes/menu-items/pages.

I have to admit that some of the scenarios around routes<=>short-codes make my head hurt, so I don't claim to fully understand them. However, I'm a little concerned this is integrating from the wrong angle. So bear with me while I try to verbalize my understanding of a current situation -- and describe then how this is a problem.

Context: Civi Pages<=> WP Shortcodes: I understand the difficulty in Civi-WP short-codes to be the consequence of impedance mismatch. Civi-WP maps between two things (a "page"/"route"/"menu-item" in Civi and a "shortcode" in WP). These are similar in that both output a blob of HTML. The difference:

  • A Civi page/route/menu-item is scoped as a primary/central thing on a screen, and it canonically drives page-lifecycle (handling submissions/buttons/validations/etc).
  • A shortcode is scoped as mixable thing (where you can have many side-by-side), and it canonically outputts a nicer widget as part of somebody else's page-lifecycle.

To expose standalone pages as mixable shortcodes, the Civi-WP short-code integration has fiddly-bits like "hijack"ing.

Context: Civi Pages<=>Afforms: For Afform, we had a similar problem, but going in the other direction:

  • An Afform (like any Angular directive or Angular component) is scoped as a mixable thing (where you can have many side-by-side).
  • But a Civi page/route/menu-item is scoped as a primary/central thing.

To expose mixable afforms as standalone pages, the afform extension has fiddly-bits like "afformStandalone" (which sets up a standalone page and then loads a single afform into it).

For CRM_Afform_Page_AfformBase, this is how "afformStandalone" wires it up:

// AfformBase.php
$loader->setModules([$afform['module_name'], 'afformStandalone']);
...
$loader->getRes()->addSetting([
  'afform' => [
    'open' => $afform['directive_name'],
  ],
]);
<!-- AfformBase.tpl -->
<div ng-app="afformStandalone">...</div>
// AfformStandalone.js
template: function() {
    return '<div id="bootstrap-theme"><' + CRM.afform.open + '></' + CRM.afform.open + '></div>';
}

Issue (Lower-level/symptomatic POV): If a page includes two short-codes which reference different afforms, then afformStandalone would bungle them up. (At best, you'd probably get two copies of the second form.)

Issue (Higher-level POV): It looks like this PR forms a bridge along this 3-part path:

Mixable Afform =(1)=> Standalone Civi page =(2)=> Mixable Shortcode

This raises a few mismatches:

  • Step (1) munges things to get rid of mixability, and step (2) munges things to create mixability. If you step into this as a new developer (or as an old developer 6 months later), the complexity is higher because it's double-munged.
  • The JS/data bundles that are prepared for the two copies would likely be working at-odds as well.
  • afformStandalone tees-up ngRoute. But if you can have 2+ subforms, then it is not sensible for each of them to lay claim to ngRoute. In fact, afform hasn't really been using ngRoute in a meaningful way. (IIRC, coleman previously raised the idea of removing it from afform).

Alternative (sketch): My gut says:

  1. For afform<=>shortcode, we should use a direct/2-layer mapping which skips afformStandalone, ngRoute, routes/menu-items, and q.
  2. When rendering a WP page with some afform-based short-codes:
    • In processing the PHP request, there should be a call to civicrm_initialize()
    • Each afform/shortcode should output a snippet of HTML like <div ng-app><my-user-selected-form/></div> in the spot where it is to be displayed.
    • Each afform/shortcode should make a call to $angularLoader->addModules('myUserSelectedForm');
    • In processing the PHP request there should be one copy of AngularLoader shared by all shortcodes. This is responsible for registering (consistent/non-duplicated) <script> tags.

@seamuslee001
Copy link
Contributor

The code looks like it is heading in the right direction and I agree with @totten's thoughts here I think.

@christianwach
Copy link
Member

I have to admit that some of the scenarios around routes<=>short-codes make my head hurt, so I don't claim to fully understand them.

@totten Sounds like you've got a pretty good handle on them to me :)

colemanw added a commit to colemanw/civicrm-wordpress that referenced this pull request Apr 6, 2021
This allows civi core or extensions to supply content for shortcodes without rendering a page;
they can simply implement `civicrm_shortcode_get_content` and return the content.

This new filter is called whenever displaying contents of a pathless shortcode.

See civicrm/civicrm-core#19687
@colemanw
Copy link
Member Author

colemanw commented Apr 7, 2021

I've rebased this to reflect the feedback from @christianwach and @totten - it now works the way Tim described, independently of Civi page routing, and consequently it's not restricted to one shortcode per page.

Note this depends on civicrm/civicrm-wordpress#244

@eileenmcnaughton
Copy link
Contributor

Nice - it would be REALLY good to get this merged before we cut the rc

Adds a v3 Afform api solely for the purpose of enabling entityRef widgets, which use APIv3.
This implements the necessary hooks & callbacks for picking an afform from the Civi shortcode popup,
and displaying the afform when viewing a single WP post/page or list of posts.

Note that unlike traditional Civi shortcodes, afform does not rely on invoking a civi page request,
but simply outputs the AngularJS directive.
@kcristiano
Copy link
Member

commenting at civicrm/civicrm-wordpress#244

@eileenmcnaughton
Copy link
Contributor

Per discussion on the other - ok to merge now to be in the rc but any changes based on further feedback can also go into the rc

@eileenmcnaughton eileenmcnaughton merged commit 73a5a9a into civicrm:master Apr 7, 2021
@eileenmcnaughton eileenmcnaughton deleted the afformShortcodes branch April 7, 2021 19:34
@agileware-justin
Copy link
Contributor

@flantascience
Copy link

Is there a way to use these shortcodes on things like the Petition Thank You page? When I enter a shortcode to displpay a SearchKit form, I get an error "This Shortcode could not be handled. It could be malformed or used incorrectly."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants