"Accordion" display broken due to missing jQuery Accordion dependency

Created on 31 May 2023, over 1 year ago
Updated 20 August 2024, 3 months ago

Problem/Motivation

The Accordion widget seems to be currently broken for both the "Olivero" and the "Claro" theme.

The form display:

The form:

There are NO JavaScript errors in the console, nor does setting the Accordion "Effect" config to "Bounce Slide" change anything.

I also tried clearing all browser and Drupal caches, but nothing. I have not tried any other themes, but as both Olivero and Claro are Drupal default themes, they should work as intended. Furthermore, no issues for this are documented in the README.md.

Steps to reproduce

  • Enable the module
  • Manage Article Form display
  • Create an "Accordion" and two "Accordion Item" field group and structure them as required (see screenshot above).
  • Check the Form

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Fixed

Version

3.0

Component

Miscellaneous

Created by

πŸ‡©πŸ‡ͺGermany Grevil

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Grevil
  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Configuration in the 1st Screenshot looks correct! Can anyone confirm this? Perhaps on a different form or theme?

  • πŸ‡§πŸ‡ΎBelarus q2_faith

    @anybody, I can confirm. You can easily check it on https://simplytest.me/

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Thanks, so someone should find out the reasons and fix it. Thank you!

  • πŸ‡§πŸ‡ΎBelarus q2_faith

    Hi there,

    Need to install module jquery_ui_accordion β†’ bexause now it's contrib module.

  • Status changed to Needs work over 1 year ago
  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @q2_faith: Thank you! That means, to fix this, we should separate the accordion feature into a separate submodule and add a dependency on jquery_ui_accordion module in that submodule (besides field_group dependency).

    We should not add that dependency in the field_group module, as it would then require everyone using field_group to also install jquery_ui_accordion, even if not used.
    Furthermore, it will need a detection, if a field group accordion is used in the project. If yes, the update hook should enable that submodule.
    An alternative would be to enable it by default and inform the user in the update hook, that it can be removed, if not used. But I think that's far less convenient.

    As jQuery was removed in Drupal 10, we could discuss creating a separate 4.x branch for that.

    Any maintainer feedback on the plan, please?

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    2 pass, 6 fail
  • @anybody opened merge request.
  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Interesting, at least in the 8.x-3.x-dev branch the libraries file already contains

    formatter.accordion:
      js:
        formatters/accordion/accordion.js: {}
      dependencies:
        - core/jquery
        - core/once
        - jquery_ui_accordion/accordion

    while the module file didn't contain that dependency.

    I added that dependency in the quickfix branch. Here's the patch, if anyone needs it:
    https://git.drupalcode.org/project/field_group/-/merge_requests/35.diff

    For the real solution we should now discuss #7 with the maintainers.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    2 pass, 11 fail
  • Status changed to Needs review over 1 year ago
  • πŸ‡©πŸ‡ͺGermany Grevil

    I added the module to the test dependency, so it is clear, that this change does NOT lead to failing tests. This can be reverted later, if the maintainer decides to put 'Accordion' in its own submodule.

    (Setting this to Needs Review, so this issue becomes more noticable).

  • πŸ‡©πŸ‡ͺGermany Grevil

    Also, there seems to be an incomplete and / or risky test. I created a separate issue for that: πŸ“Œ Fix risky test Active .

  • πŸ‡§πŸ‡ΎBelarus q2_faith

    We should not add that dependency in the field_group module, as it would then require everyone using field_group to also install jquery_ui_accordion, even if not used.

    Totally agree.

    Interestingly that the module checks this additional module but I don't see any messages.

  • πŸ‡©πŸ‡ͺGermany Grevil

    Oh well, tests still fail, I probably also need to adjust the test module dependencies, but not really worth it, as the location for the accordion will change anyway.

  • πŸ‡ͺπŸ‡ΈSpain candelas

    Thanks @q2_faith !!
    I have installed module jquery_ui_accordion and now works!

    Maybe you could write this in the README.md and in the front page...

    Thanks a lot for the module :)

  • πŸ‡«πŸ‡·France pbonnefoi

    I think accordion should be removed from the module and be in another sub mobule. It's also consufing because "Detail" field group is also like an accordion.

  • Assigned to KostasTs
  • Issue was unassigned.
  • Status changed to Closed: works as designed 8 months ago
  • πŸ‡¬πŸ‡·Greece KostasTs

    @Grevil The patch seems to solve the issue.

  • Status changed to RTBC 8 months ago
  • πŸ‡©πŸ‡ͺGermany Grevil

    @Anybody, should we merge this then as a quick fix for the time being or properly dissect the module and move the accordion part in its own submodule in this issue?

  • πŸ‡¬πŸ‡·Greece vensires

    I personally think that if we moved this functionality away, it will be like we deprecate it since it's been here for years and developers expect it to exist by default with the Field Group module.

    Unless you do want to deprecate it; that's another discussion...

  • Status changed to Needs work 6 months ago
  • πŸ‡§πŸ‡ͺBelgium nils.destoop

    This patch adds the jquery_ui_accordion as hard dependency, and that is not what we want.
    There is a hook_requirements implementation to warn that you need to install the jquery_ui_accordion module if you want to use accordions.

    However, marking it as deprecated is a good solution. As you don't want to use it anymore, since Jquery UI is EOL.
    I'll change 2 things:

    • Change the requirements implementation to a warning, so it is on top of the reports page. Now it was only an info item
    • Add (deprecated) to the name of Accordion and also add a warning on the reports page if you are using this formatter
    • 771352bf committed on 8.x-3.x
      #3363859: Move accordion to it's own module and install if people are...
  • Status changed to Fixed 6 months ago
  • πŸ‡§πŸ‡ͺBelgium nils.destoop

    I went for the submodule approach. That module is now also marked as deprecated and a warning will appear on the reports screen.
    An update hook was written to handle the install on existing sites.

  • πŸ‡«πŸ‡·France pbonnefoi

    Great work @nils.destoop !

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

    What is the recommended, non-deprecated, replacement for accordion?

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

    Also looking for recommendations for a replacement.

  • πŸ‡«πŸ‡·France pbonnefoi

    I would say replace accordion field group by detail.

  • πŸ‡³πŸ‡±Netherlands Jan-E

    @nils.destoop in #22 πŸ› "Accordion" display broken due to missing jQuery Accordion dependency Fixed

    However, marking it as deprecated is a good solution. As you don't want to use it anymore, since Jquery UI is EOL.

    Exactly what of Jquery UI is EOL? 15 days ago there was a release (1.14.0):
    https://blog.jqueryui.com/category/releases/

Production build 0.71.5 2024