Closed (works as designed)
Project:
Bootstrap Paragraphs
Version:
8.x-1.0-beta1
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
29 Mar 2017 at 18:38 UTC
Updated:
3 Apr 2017 at 21:11 UTC
Jump to comment: Most recent
Comments
Comment #2
brandonratz commentedI believe a better solution is to fix the markup in 'paragraph--accordion.html.twig'. There is an unnecessary wrapper with the class 'panel-title'. This div.panel-title could be removed and the class 'panel-title' added to the anchor tag.
Change this (line 153):
To this:
Also notice I am using |striptags filter on the title because of a related issue.
Comment #3
thejimbirch commentedI like the idea of the display: block on the Accordion title.
While your markup is simpler, I don't agree that the title div is unnecessary. That div comes from the Card (Bootstrap 4) and Panel (Bootstrap 3) markup that is shown in the Accordion example of the Bootstrap Collapse Javascript.
http://getbootstrap.com/javascript/#collapse-example-accordion
https://v4-alpha.getbootstrap.com/components/collapse/
Comment #4
brandonratz commentedThank you for pointing this out Jim. I'm going to agree to disagree (sort of*) and also state that I don't have a large enough Bootstrap experience to know what is "best practice".
The panel-title class applies the exact styles we're looking for, ie display: block. *The intent of .panel-title, imo, is to provide a wrapper element which spans the full width of the panel. When you do not need this wrapper (ie H3/H4 from example code) it is unnecessary. It adds additional markup and the need for additional styles to accomplish the same thing. Of course, if you do want the clickable accordion title to be a heading for example, it is needed.
In short, I believe we are both right. If it is desirable to remain in the Bootstrap universe as much as possible, it should be the end user's choice to add the styles, not the module. To apply display: block to the inner anchor element is an opinionated decision and if Bootstrap itself doesn't employ this practice, the module should not.
Comment #5
thejimbirch commentedGreat points. I think for now we should stick with the Bootstrap demo code and re-address if more people bring it up.