Grouping support for Item and Email FormElements

Created on 24 March 2020, over 4 years ago
Updated 14 June 2024, 14 days ago

Problem/Motivation

The Item and Email @FormElement elements do not support grouping (using the field_group module for example).

Steps to reproduce

1. Enable the contact module and the field_group module from contrib
2. Create a contact form and add a group to the form display page.
3. Nest the e-mail field into the group created in the step above.
4. View the form. The e-mail field is rendered outside of the group.

Proposed resolution

Add the 'processGroup' callback to the #process info attribute of the E-mail @FormElement. It seemed that more @FormElement plugins were missing the 'processGroup' callback, so add them there too.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

Enable group processing for FormElement plugins where missing.

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
RenderΒ  β†’

Last updated about 7 hours ago

Created by

πŸ‡³πŸ‡±Netherlands Ruuds

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Was previously tagged for tests in #6 which still needs to happen please.

  • πŸ‡§πŸ‡ͺBelgium Mav_fly

    I also can confirm that the patch provided in #5 fixes the bug.
    My setup:
    Drupal 10.2.4
    Field Group 8.x-3.4
    Account field split 2.0.2

  • Pipeline finished with Failed
    3 months ago
    Total: 100s
    #125268
  • Pipeline finished with Failed
    3 months ago
    Total: 176s
    #125271
  • Pipeline finished with Failed
    3 months ago
    Total: 481s
    #125276
  • Status changed to Needs review 3 months ago
  • πŸ‡³πŸ‡±Netherlands Ruuds

    So as the issue only seems to occur when grouping elements using the field_group module, do we really want to rebuild the logic that field_group uses for grouping elements into a core test? Creating a dependency to the field_group module for a specific core test doe also not feel right.

    I've created a MR which contains a test which checks if all FormElement plugins defined by core have a #process callback to processGroup, which solves the problem. Running that test, I discovered additional FormElements which also did not have the #process callback.

    Probably work in this MR will be removed again when https://www.drupal.org/project/drupal/issues/2190333 πŸ“Œ Make #group FAPI / render feature work on all form/render #types out of the box Needs work gets fixed.

  • Pipeline finished with Canceled
    3 months ago
    #125291
  • Pipeline finished with Failed
    3 months ago
    #125297
  • Pipeline finished with Failed
    3 months ago
    Total: 496s
    #125314
  • Pipeline finished with Running
    3 months ago
    #125325
  • Pipeline finished with Failed
    3 months ago
    Total: 519s
    #125332
  • Pipeline finished with Success
    3 months ago
    #125343
  • Status changed to Needs work 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Issue summary needs to be updated. I added the template but left the sections empty. Main ones needed are steps to reproduce and proposed solution. If no UI or date model change NA is fine.

    After that probably can ask sub-maintainer for review of solution.

  • πŸ‡³πŸ‡±Netherlands Ruuds
  • Status changed to Needs review 3 months ago
  • πŸ‡³πŸ‡±Netherlands Ruuds
  • Pipeline finished with Success
    3 months ago
    Total: 494s
    #129502
  • Status changed to Needs work 3 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • Status changed to Needs review 3 months ago
  • πŸ‡³πŸ‡±Netherlands Ruuds

    I don't think anything needs to be changed? The mentioned class (Drupal\Core\Render\Element\FormElementBase) by the needs-review-queue-bot does not exist and FormElement is not marked as deprecated?

  • Status changed to Needs work 3 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • patches don't work for drupal 10.2.7 :(

Production build 0.69.0 2024