search

backdrop-contrib / paragraphs_jquery_ui_accordion

An accordion display for paragraph title and description content on Backdrop CMS
https://backdropcms.org/project/paragraphs_jquery_ui_accordion
GNU General Public License v2.0
1 stars 4 forks source link

Multiple nested accordion paragraphs nested in a single paragraphs field don't work #11

Closed yorkshire-pudding closed 1 year ago

yorkshire-pudding commented 1 year ago

Steps to reproduce

  1. Add a paragraph type that has a paragraph field
  2. Allow all paragraph types that are suitable for use in an accordion
  3. Set display mode for field to be 'Paragraphs jQuery UI Accordion 2'
  4. Ensure that paragraph type is allowed for a paragraph field on a content type
  5. Create new content and add paragraphs including two of the new type you created (it's easier to see if you add other paragraph types in between.

Expected result All accordion paragraphs work

Actual result Only first accordion paragraph works

The PR added as the fix for https://github.com/backdrop-contrib/paragraphs_jquery_ui_accordion/issues/7 while fixing the two fields use case broke it for 2 accordions in a single field.

The solution is to have the accordion id made of both the field name and the entity id. In this case, the entity id becomes the paragraph item.

This is the same as https://github.com/backdrop-contrib/paragraphs_jquery_ui_accordion2/issues/2

yorkshire-pudding commented 1 year ago

@bdalomgir and @stpaultim I have tested this against your use case in #7 and it works there as well as fixing the use case here. Please either test and then confirm, or confirm that you've seen this and are happy on the basis of my tests.

@alanmels - FYI

alanmels commented 1 year ago

@yorkshire-pudding, I believe you have a maintainer privilege for this module, so if your fix works for both use cases, please go ahead and make necessary changes. Unfortunately, I don't have time to test this.

yorkshire-pudding commented 1 year ago

Yes, I do; just wanted to keep you in the loop and give the stakeholders of issue #7 an opportunity to test. I've merged in the other module and it works, but I think I'm the only one using that module so far judging by usage stats. I'll give @bdalomgir and @stpaultim a couple of days and then merge and release.

bdalomgir commented 1 year ago

@yorkshire-pudding I have checked your PR and I confirmed this PR also works for https://github.com/backdrop-contrib/paragraphs_jquery_ui_accordion/issues/7 use case so please proceed.