Improve content form detection

What about adding a hook to _gin_is_content_form() so that other modules can opt-in to use the gin layouting?

+1 for this. Just noticed this after generating two custom content entities with Drush.

What about adding a hook to _gin_is_content_form() so that other modules can opt-in to use the gin layouting?

I think as well that this is the way to go as it is a much cleaner and easier solution as there is not an universal pattern in which cases we would want to support the Gin Edit form layout.

Uploading a patch with that solution. I've added only a hook to collect routes, but we might also add one to alter the routes.

I agree in it being beneficial for this to utilize a hook_*_alter() implementation to add the ability to remove routes as well.

Adding the alter hook.

In testing this out I found that pieces were needed from both the patch from #1 and the patch from #8 so I started a new issue fork that has both of these changes.

I added a final commit that enables the node content form for taxonomy terms. This is very simple and only for demonstration purposes! It adds some simple text and moves the relations fieldset to the sidebar. I've opened a MR to trigger the tugboat preview that demonstrates this: https://mr40-fdz2e5aj1kymtu7eronixscbhuy3knle.tugboat.qa/admin/structure...

Overall, these changes are pretty simple and would make it much easier for other modules providing custom entity types to use the node edit form. The hook_gin_content_form_routes works great although I'm curious if this should be implemented as an event instead? This might be a bit more future-proof and allow for other options such as enabling the node edit form for all forms by id or base_id (similar to existing logic already implemented in _gin_is_content_form()).

It's also worth nothing that while this enables custom entity types to use the node edit form, this stillrequires that the node module is installed. We don't require the node module for farmOS, but are hoping to use the same layout of the node edit form. Regardless, this issue is a great first step towards supporting custom entity types!

I'm not sure how possible it would be to decouple this from the node module... Gin is dependent on the meta form element which is provided by the NodeForm::form method. The patch from #1 changes this because obviously custom entity types can't be expected to extend from the NodeForm. But it also seems like Gin is providing most (but not all?) of the styling for the layout-region-node-secondary, and obviously is using the "node" in the name of these classes.. So maybe the CSS/JS (the claro/node-form library, and it's dependency on node/form are the only remaining implicit dependencies on the node module? Does it seem possible to abstract this out? If possible, this should probably be tackled in a separate issue.

Moved functionality to a "service" class.

Looks good. I think for terms we should create a separate issue.

I move this back to needs review as there were changes added :)

While all the work here is useful, I believe the theme should handle these forms out-of-the box, without needing opt-in.

Why not do something like this:

    if (substr($route_name, 0, 7) === 'entity.') {
      if (substr($route_name, -9) === '.add_form') {
        $is_content_form = TRUE;
      }

      if (substr($route_name, -10) === '.edit_form') {
        $is_content_form = TRUE;
      }
    }

The issue https://www.drupal.org/project/gin/issues/3231922 will need to be fixed for the edit form to work in some forms, adding as related.

Hey,

thanks a lot for the great work and a very cool theme :). I just came across this same issue and did some research before finding this issue.

I have looked at the patches and read the issue and have a few thoughts. First of all - I don't think it is good to use the node form template and it's classes. Styling for the `layout-region-node-secondary` sits in stable theme in node.module.css. Maybe we could just take them from there and use some more generic class name instead? Or even better (tho it might take some time till it gets accepted :D) - make a patch to stable theme with such generic classes (for example `layout-region-entity-secondary`).

This would require us to register a new theme hook - entity_edit_form and use it instead of the node one. This could be configurable via UI - we could exclude or include manually specific entity type (tho that's a nice to have I think ;)).

I also think we could adjust the `isContentForm` method a bit to allow for a better autodetection. Below just a quick sketch:

public function isContentForm(array $form = NULL, FormStateInterface $form_state = NULL, $form_id = NULL) {
    $forms_to_exclude = $this->getFormsToExclude();
    // formIsExcluded would be a helper method with the exclude logics.
    if ($this->formIsExcluded($form_id) {
      return FALSE;
    }

    $auto_detect_content_forms = // Somehow get gin setting telling whether we should include all content forms by default
    // Here in routeIsIncluded all the logics already written for route based detection - if that returns true no need for further processing
    if ($this->routeIsIncluded()) {
      return TRUE;
    }

     // If not returned so far, and auto detection is disabled - return FALSE
    if (!$auto_detect_content_forms) {
      return FALSE;
    }


    if ($form_state) {
      // Check if form implements the ContentEntityFormInterface. Or EntityFormInterface if we need config entities as well.
      return $form_state->getFormObject() instanceof \Drupal\Core\Entity\ContentEntityFormInterface;
    }

  $route_object = \Drupal::routeMatch()->getRouteObject();
  if ($route_object) {
    // This handles all routes that use Drupal's _entity_form
    $formHandler = $route_object->getDefault('_entity_form');
    if ($formHandler) {
      try {
        $form = \Drupal::entityTypeManager()->getFormObject(...explode('.', $formHandler));
        return $form instanceof \Drupal\Core\Entity\ContentEntityFormInterface;
      }
      // Make the exceptions more specific - maybe log.
      catch (Exception $e) {
        return FALSE;
      }
    }
  }
  }

That's basically just a rough idea written quickly. I'll try to make a patch when I have a few moments if this approach makes sense.

I've just figured out how to request push access and made the suggested change, as well as improved the examples.

I also was unable to make a useful clean patch file from https://git.drupalcode.org/project/gin/-/merge_requests/40.patch
due to the GinContentform.php being renamed to GinContentFormHelper.php it somehow left a dirty file when applying the patch with composer, so here is a patch of work to-date based on the diff between the MR branch and 8.x-3.x branch after a rebase.

https://git.drupalcode.org/issue/gin-3188521/-/compare/8.x-3.x...3188521...

Edit. I later figured out that https://git.drupalcode.org/project/gin/-/merge_requests/40.diff produced a unified diff file, which is better than the patch file mentioned in Drupal docs, as it doesn't try to reapply intermediate commits in the MR, one on top of the other, like the patch file does.

Tested with the Box module. Form and color changes working nicely.

Looks good! The only real change I noticed is that the content_lock unlock button moved to the sidebar, but that's okay.

tested this by using the hooks to enable meta on 'entity.taxonomy_term.edit_form' and 'entity.taxonomy_term.add_form'.

We should add a follow-up to enable this on taxonomy terms by default (and move relations details into meta).

I think this is good to go.

Thanks to everybody who participated in this!

These changes are merged to dev and beta1 released before reverting commit 90da47. Currently action buttons are not showing anymore in the taxonomy forms. I created a follow-up ticket on this https://www.drupal.org/project/gin/issues/3270626

@meladawy fyi: that was never part of Gin (yet) and was for testing purposes only. So it is correct how it was merged.

@saschaeggi so do you expect any websites using beta1 version to use the hooks to enable the actions on taxonomy forms?