Grouping support for Item and Email FormElements

Created on 24 March 2020, about 5 years ago
Updated 1 February 2023, about 2 years ago

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

See the attached patch which fixes this.

πŸ› Bug report
Status

Needs work

Version

10.1 ✨

Component
RenderΒ  β†’

Last updated 2 days 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.

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
    about 1 year ago
    Total: 100s
    #125268
  • Pipeline finished with Failed
    about 1 year ago
    Total: 176s
    #125271
  • Pipeline finished with Failed
    about 1 year ago
    Total: 481s
    #125276
  • Status changed to Needs review about 1 year 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
    about 1 year ago
    #125291
  • Pipeline finished with Failed
    about 1 year ago
    #125297
  • Pipeline finished with Failed
    about 1 year ago
    Total: 496s
    #125314
  • Pipeline finished with Running
    about 1 year ago
    #125325
  • Pipeline finished with Failed
    about 1 year ago
    Total: 519s
    #125332
  • Pipeline finished with Success
    about 1 year ago
    #125343
  • Status changed to Needs work about 1 year 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 about 1 year ago
  • πŸ‡³πŸ‡±Netherlands Ruuds
  • Pipeline finished with Success
    about 1 year ago
    Total: 494s
    #129502
  • Status changed to Needs work about 1 year 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 about 1 year 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 about 1 year 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 :(

  • πŸ‡ΊπŸ‡¦Ukraine vitalius2009

    Patch version for drupal 10.4.0

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 1155s
    #393037
  • πŸ‡ͺπŸ‡ΈSpain vidorado LogroΓ±o (La Rioja)

    Rebased onto latest 11.x branch.

    Multiple tests are failing now, but I haven't managed to fix them. RenderElementBase::preRenderGroup and RenderElementBase::preRenderGroup seem to do something that makes some elements don't appear on the page on functional tests.

  • πŸ‡ͺπŸ‡ΈSpain vidorado LogroΓ±o (La Rioja)
  • Pipeline finished with Failed
    3 months ago
    Total: 3255s
    #393070
Production build 0.71.5 2024