The person I worked for had some suggestions for how they wanted their accordions/changes that need to be done to pass our 508 testing. I did some major tweaks to it and hope it can be useful to others.

Changes I did include:

  • Added an Expand All/Collapse All button above each accordion group. Which required tweaking the accordion twig and css files
  • Added a plus/minus indicator for each accordion for visual indication. These probably should be images but that can be looked at later.
  • Added a javascript file to be used for the accordion. Have alternating alt text for the Expand/All and accordion buttons so that a screen reader will be able to tell which is open or closed.
  • Changed it so the whole panel-title can be clicked opposed to the title, thought this would help with mobile
  • Created a custom field formatter for the accordion so we an admin can select if they want the first tab defaulted to open or not. Got the idea from (https://www.drupal.org/node/2890307)

Know this patch needs some work just wanted to get the new ideas out there for some feedback.

I'm relatively new to Drupal 8 so hope what I got isn't too bad!

Added 2 images to show what the config looks like and how it renders.

CommentFileSizeAuthor
#15 bootstrap-paragraphs_accordion-updates_2920906-15.patch36.9 KBsmustgrave
#6 bootstrap-paragraphs_accordion-updates_2920906-6.patch34.57 KBsmustgrave
#4 features display example.PNG123.2 KBsmustgrave
#4 features accordion display config.PNG26.7 KBsmustgrave
#2 bootstrap-paragraphs_accordion-updates_2920906-2.patch22.09 KBsmustgrave

Comments

smustgrave created an issue. See original summary.

smustgrave’s picture

smustgrave commented
Version: 8.x-2.0-beta2 » 8.x-2.x-dev
Issue summary: View changes
StatusFileSize
new bootstrap-paragraphs_accordion-updates_2920906-2.patch22.09 KB
smustgrave’s picture

smustgrave commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Issue summary: View changes
StatusFileSize
new features accordion display config.PNG26.7 KB
new features display example.PNG123.2 KB
thejimbirch’s picture

thejimbirch commented

Hi Stephen,

Thanks for the contribution and patch! I will review this week and post back.

Jim

smustgrave’s picture

smustgrave commented
StatusFileSize
new bootstrap-paragraphs_accordion-updates_2920906-6.patch34.57 KB

So I had some time of my hands and decided to come at this from another approach. Since some users may want to keep the original accordions I decided to just create a new paragraph type and use my changes there. Let me know if anyone has any thoughts about it.

mdrescher’s picture

mdrescher commented

Love it!

Reviewed the patch intellectually - I have never embarked on writing modules etc myself, but I sort of understand how it works.

A few suggestions perhaps, as configuration improvements:

  1. Make the glyph icon pair for expanded/collapsed configurable, e.g. the following sets:
    • glyphicon-plus / glyphicon-minus
    • glyphicon-chevron-right / glyphicon-chevron-down
    • glyphicon-menu-right / glyphicon-menu-down
    • glyphicon-triangle-right / glyphicon-triangle-bottom
  2. Make the position of the glyphicon set configurable to left or right of the accordion section title
smustgrave’s picture

smustgrave commented

@mdrescher I like the idea of those configuration changes a lot. Unfortunately I may not be able to get around to them this week but definitely something I will play around with. Before I do that though @thejimbirch is this patch something you would consider adding into a future release?

thejimbirch’s picture

thejimbirch commented

@smustgrave

Sorry for the delay and again for the A11Y review and the patch. I've had a bit of a chance to review, but not implement.

I love:

  • Added an Expand All/Collapse All button above each accordion group.
  • Added a javascript file to be used for the accordion. Have alternating alt text for the Expand/All and accordion buttons so that a screen reader will be able to tell which is open or closed.
  • Changed it so the whole panel-title can be clicked opposed to the title, thought this would help with mobile

For the Added a plus/minus indicator for each accordion for visual indication, I'd rather see use move to using text rather than glyphicons since they have been removed from Bootstrap 4. The same approach could be used using +, -, ^, ^ (rotated) instead. I don't think an icon position is needed. That could easily be changed in the theme's CSS.

For the Created a custom field formatter for the accordion so we an admin can select if they want the first tab defaulted to open or not., I'd rather see us put another select field that sets that in the Template. This would be similar to how we set widths, carousel speed etc... That way the content creator would be able to choose that option on an accordion by accordion basis, not just on all or not.

I'll see if I can get some of this commited in the next week or so, then we can circle back and review/add more.

Thanks all!

mdrescher’s picture

mdrescher commented

@thejimbirch, @smustgrave,

Re glyphicon vs text I am in favour of using icon fonts - if not glyph then one of the Bootstrap recommended ones, e.g. https://useiconic.com/open/. Using text would IMHO make the accordion susceptible to font choices of the designer - being a graphican design element I think accordion would suffer from text symbols.

I second the argument for using fields for tabs being open or not.

  • smustgrave authored aa7721c on 8.x-2.x 
    Issue #2920906 by smustgrave, thejimbirch: Multiple changes to accordion
thejimbirch’s picture

thejimbirch commented
Status: Needs review » Fixed

Merged in:

  • Added an Expand All/Collapse All button above each accordion group.
  • Added a javascript file to be used for the accordion. Have alternating alt text for the Expand/All and accordion buttons so that a screen reader will be able to tell which is open or closed.
  • Changed it so the whole panel-title can be clicked opposed to the title, thought this would help with mobile
  • Added a plus/minus indicator for each accordion for visual indication (using text)

@smustgrave Can you help comment what the scripts do in the JavaScript file? Is there anything in there related to the field formatter for expansion? If so, it should be removed and we can address in Add option to Accordion to be initially expanded.

smustgrave’s picture

smustgrave commented

Sure do you want me to create another patch with the comments? The javascript file only handles the opening/closing of the accordion and changing any atl text for a screen reader.

thejimbirch’s picture

thejimbirch commented

Thanks @smustgrave. I got a lot of what the functions do, but there were some that I didn't get.

You can make a patch, or comment in the file, upload and I can add.

smustgrave’s picture

smustgrave commented
StatusFileSize
new bootstrap-paragraphs_accordion-updates_2920906-15.patch36.9 KB

@thejimbirch let me know how this looks. Sorry for the delay.

  • smustgrave authored ef243dc on 8.x-2.x 
    Issue #2920906 by smustgrave, thejimbirch: Adds comments into accordion...
thejimbirch’s picture

thejimbirch commented

Thanks @smustgrave! The comments are in dev now, will be added on the next release.

smustgrave’s picture

smustgrave commented

@thejimbirch I downloaded the dev version and I see the javascript file but nothing else from the patch is that correct?

thejimbirch’s picture

thejimbirch commented

@smustgrave In #11 I commited everything from your patch but that additional field formatter. It should be in beta3 and dev.

If you are working on the other issue, you should create a new patch from dev.

Thanks!

smustgrave’s picture

smustgrave commented

@thejimbirch I checked beta3 and dev and only thing I see added is the javascript file, everything else is missing.

thejimbirch’s picture

thejimbirch commented

Not sure what you are missing.

Here is the template file with your changes:

https://cgit.drupalcode.org/bootstrap_paragraphs/tree/templates/paragrap...

The less file:

https://cgit.drupalcode.org/bootstrap_paragraphs/tree/less/bootstrap-par...

The css file:

https://cgit.drupalcode.org/bootstrap_paragraphs/tree/css/bootstrap-para...

smustgrave’s picture

smustgrave commented

Those aren't the changes from the patch.

thejimbirch’s picture

thejimbirch commented

Can you please be more specific?

smustgrave’s picture

smustgrave commented

Yup sorry, I'm not seeing the formatter, the template changes don't appear to be mine because I used a second template all together. I don't see the config files I made for the accordion-v2 type. The javascript file is now named bootstrap-paragraphs-accordion.js but if you look in the patch I called it bootstrap-paragraphs-accordion-v2.js.

If the goal is to get accordions to work like discussed in https://www.drupal.org/project/bootstrap_paragraphs/issues/2890307 then these changes won't be needed but thought I'd point out that they're currently not appearing.

thejimbirch’s picture

thejimbirch commented

Correct, in #12, I manually merged the JS, LESS/CSS, and template changes from your patch into the Accordion bundle. Having a second Accordion bundle would be difficult to maintain and administer. I did not merge the field formatter as I think it should be on a per entity basis like we are working on in #2890307 - Add option to Accordion to be initially expanded.

With your changes for A11Y in this issue, and the additional functionality discussed in #2890307 - Add option to Accordion to be initially expanded we should have an awesome Accordion that should cover a lot of uses.

Status: Fixed » Closed (fixed)

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

dunebl’s picture

dunebl
Primary language French
commented
Category: Feature request » Bug report

Downloading the last version of this module, I discover that the button "Expand All" doesn't work anymore.(D8.5)
Should I create a bug or this tread can hold this problem?

thejimbirch’s picture

thejimbirch commented

@DuneBL I believe it is only in the dev branch. I will try to make an updated release soon.

smustgrave’s picture

smustgrave commented

Believe we have 2 versions of the same patch floating around between this ticket and https://www.drupal.org/project/bootstrap_paragraphs/issues/2890307. I've since rerolled the patch and applied it to the other ticket.

dunebl’s picture

dunebl
Primary language French
commented

@thejimbirch you are right, using 8.x-2.0-beta3 solve this issues

smustgrave’s picture

smustgrave commented

@DuneBL and @jimbirch can you guys try the patch I posted to the other ticket?

dunebl’s picture

dunebl
Primary language French
commented

@smustgrave I have downloaded the dev version and installed [2890307], unfortunately, the "expand all" button doesn't work after that...

smustgrave’s picture

smustgrave commented

@DuneBL just uploaded a new patch to that ticket if you want to try that out. There was some overlap and gaps between the patches since it was on two tickets.

dunebl’s picture

dunebl
Primary language French
commented

@smustgrave This is perfect! You solve this issue.
Many thanks!!