Add an InlineForm API, stop using complex form elements

+1,000,000

In core we have \Drupal\Core\Form\SubformState - but I think this is more forms inside of forms.

First, this is great. And amazing how simple it looks.

1) I agree. No need for the third parameter. It can be fetched via the form state. It's passed by reference (for better or worse) anyways: &getCompleteForm()
2) Agree.
3) It's worth keeping less magical until we roll out more implementations, I'd say.

1. +++ b/modules/payment/src/Element/PaymentGatewayForm.php
   @@ -3,15 +3,15 @@
   + * @deprecated Use the payment_gateway_form inline form instead.
   + *
   

   Should processForm throw a trigger_error, or however core logs deprecation notices

2. +++ b/modules/payment/src/Form/PaymentAddForm.php
   @@ -26,6 +27,13 @@ class PaymentAddForm extends FormBase implements ContainerInjectionInterface {
   @@ -38,11 +46,14 @@ class PaymentAddForm extends FormBase implements ContainerInjectionInterface {
   

   While adjusting this, should we directly inject the messenger service, so we do not need to rely on the messenger() method which reaches into \Drupal::service() if the property is not set? Or, not. A bunch of plugins could use this. Maybe it's just a follow up to keep the diff simpler.

3. +++ b/src/Annotation/CommerceInlineForm.php
   @@ -0,0 +1,34 @@
   +   *
   +   * @ingroup plugin_translatable
   +   *
   +   * @var \Drupal\Core\Annotation\Translation
   +   */
   

   nit: phpcs says these should be flipped.

4. +++ b/src/Plugin/Commerce/InlineForm/EntityInlineFormBase.php
   @@ -0,0 +1,52 @@
   +    if (empty($this->entity)) {
   +      throw new \RuntimeException(sprintf('No entity was provided to "%s".', $this->pluginId));
   +    }
   +    if (!($this->entity instanceof $interface)) {
   +      throw new \RuntimeException(sprintf('Invalid entity provided to "%s", expecting an implementation of "%s".', $this->pluginId, $interface));
   +    }
   

   These checks are technically the same. Do we have both for verbosity in error reporting?

5. +++ b/src/Plugin/Commerce/InlineForm/EntityInlineFormInterface.php
   @@ -0,0 +1,32 @@
   +interface EntityInlineFormInterface extends InlineFormInterface {
   

   I agree, I like having this specific interface.

1. +++ b/modules/payment/src/Element/PaymentGatewayForm.php
   @@ -142,38 +124,10 @@ class PaymentGatewayForm extends RenderElement {
   +    $form_state->setValueForElement($element, $inline_form->getEntity());
   

   Are we guaranteed something that implements EntityInlineFormInterface. InlineFormInterface hints that this method might not always be available. How can we ensure that the payment_gateway_form implements this? Do we even care?

2. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -41,11 +41,11 @@ class PaymentInformation extends CheckoutPaneBase {
      /**
   -   * The messenger.
   +   * The inline form manager.
       *
   -   * @var \Drupal\Core\Messenger\MessengerInterface
   +   * @var \Drupal\commerce\InlineFormManager
       */
   -  protected $messenger;
   +  protected $inlineFormManager;
    
      /**
       * The payment options builder.
   @@ -69,16 +69,16 @@ class PaymentInformation extends CheckoutPaneBase {
   
   @@ -69,16 +69,16 @@ class PaymentInformation extends CheckoutPaneBase {
       *   The entity type manager.
       * @param \Drupal\Core\Session\AccountInterface $current_user
       *   The current user.
   -   * @param \Drupal\Core\Messenger\MessengerInterface $messenger
   -   *   The messenger.
   +   * @param \Drupal\commerce\InlineFormManager $inline_form_manager
   +   *   The inline form manager.
       * @param \Drupal\commerce_payment\PaymentOptionsBuilderInterface $payment_options_builder
       *   The payment options builder.
       */
   -  public function __construct(array $configuration, $plugin_id, $plugin_definition, CheckoutFlowInterface $checkout_flow, EntityTypeManagerInterface $entity_type_manager, AccountInterface $current_user, MessengerInterface $messenger, PaymentOptionsBuilderInterface $payment_options_builder) {
   +  public function __construct(array $configuration, $plugin_id, $plugin_definition, CheckoutFlowInterface $checkout_flow, EntityTypeManagerInterface $entity_type_manager, AccountInterface $current_user, InlineFormManager $inline_form_manager, PaymentOptionsBuilderInterface $payment_options_builder) {
   

   This type of change is always a bit tricky. It depends if you think this class is widely inherited from in contrib / custom. If it is then leaving the constructor arguments as there are and adding $inline_form_manager = NULL to the end is preferable because then you can do a BC layer.

   Wrt to using ->messenger() vs injection - I'm not sure it matters all that much but injection is preferred.

3. +++ b/modules/payment/src/Plugin/Commerce/InlineForm/PaymentGatewayForm.php
   @@ -0,0 +1,180 @@
   +  public function defaultConfiguration() {
   +    return [
   +      'operation' => NULL,
   +      // The url to which the user will be redirected if an exception is thrown
   +      // while building the form. If empty, the error will be shown inline.
   +      'exception_url' => '',
   +      'exception_message' => $this->t('An error occurred while contacting the gateway. Please try again later.'),
   +    ];
   +  }
   ...
   +      if (!empty($inline_form['#page_title'])) {
   

   Should #page_title = NULL be in the default config as a way of documenting this?

Sorry. What is the steps to use these plugins? for set a inline form with custom fields for one gateway.

Haha, I've just discovered this while making a custom private fork of Commerce Profile Panes based instead on the current implementation of the commerce customer profile. So everything is now available for alteration in the parent checkout flow form, instead of needing hook_inline_entity_form_entity_form_alter() (though you will still need #after_build callbacks on any complex form widgets within that).

A bit of code to refactor, but the result is looking totally worth it for inline entity forms in checkout panes. Many thanks.