I propose adding a style to make accordion title anchor tags display: block. This will increase the clickable area of the title improving usability.

Comments

cfbauer created an issue. See original summary.

brandonratz’s picture

I 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):

        <div class="card-header panel-heading" role="tab" id="heading-{{ key + 1 }}">
          <div class="panel-title">
            <a data-toggle="collapse" data-parent="{{ paragraph_id }}" href="#collapse-{{ key + 1 }}" aria-expanded="true" aria-controls="collapse-{{ key + 1 }}">
              {# Print the accordion section title. #}
              {{ item['#paragraph'].field_accordion_section_title.value|striptags }}
            </a>
          </div>
        </div>

To this:

        <div class="card-header panel-heading" role="tab" id="heading-{{ key + 1 }}">
          <a class="panel-title" data-toggle="collapse" data-parent="{{ paragraph_id }}" href="#collapse-{{ key + 1 }}" aria-expanded="true" aria-controls="collapse-{{ key + 1 }}">
            {# Print the accordion section title. #}
            {{ item['#paragraph'].field_accordion_section_title.value|striptags }}
          </a>
        </div>

Also notice I am using |striptags filter on the title because of a related issue.

thejimbirch’s picture

I 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/

brandonratz’s picture

Thank 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.

thejimbirch’s picture

Assigned: Unassigned » thejimbirch
Status: Active » Closed (works as designed)

Great points. I think for now we should stick with the Bootstrap demo code and re-address if more people bring it up.