Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accordion markup missing flexibility #2490

Closed
achimha opened this issue Jul 2, 2018 · 6 comments
Closed

accordion markup missing flexibility #2490

achimha opened this issue Jul 2, 2018 · 6 comments

Comments

@achimha
Copy link

achimha commented Jul 2, 2018

With the new accordion markup introduced by #2368, it is no longer possible to style the header based on its collapsed/expanded state.

Suppose one wants to add a chevron pointing right for closed and down for opened panes:

screen shot 2018-07-02 at 11 41 29

This would be appended to the h5 element like this:

h5 {
        &:after {
            font: normal normal normal 18px/1 FontAwesome;
            content: "\f054";
            margin-top: 4px;
            float: right;
            color: white;
        }

With the new markup, there is no way to get the collapsed/expanded state at the level where style needs to be applied because it is only represented on the button child element. In the previous markup, there was a a element which had the expanded style.

In my view, ng-bootstrap should apply a class at the .card-header level to allow for such styling.

@achimha
Copy link
Author

achimha commented Jul 2, 2018

Something like this would do, probably needs a different class name though
#2491

@benouat
Copy link
Member

benouat commented Jul 2, 2018

Could you show me an example of markup (both HTML & CSS) that you were using prior to the change.

To be honest I don't really see what the change introduced in #2368 has broken for you. It was the same before. We basically replaced an <a> with a <button>.

So either I am missing your problem, or maybe it was already not doable ....

@achimha
Copy link
Author

achimha commented Jul 2, 2018

Before the markup looked like this:

<ngb-accordion class="accordion" role="tablist" aria-multiselectable="true">
   <div class="card">
      <div role="tab" id="details-header" class="card-header ">
         <a href="" aria-expanded="true" aria-controls="details" aria-disabled="false">
            Title of card
        </a>
      </div>
      <div role="tabpanel" id="details" aria-labelledby="details-header" class="card-body collapse show">
       ....

The chevron from the picture above could be styled as follows:

.card-header a[aria-expanded=true]:after {
    font: 18px/1 FontAwesome;
    content: "\f078";
    margin-top: 0;
    float: right;
    color: #fff;
}

Not super-pretty by using aria-expanded but working. With the current markup, I can't attach to the h5 because only the button has an attribute that would indicate expansion.

<div class="card">
   <div role="tab" id="generaldata-header" class="card-header ">
      <h5 class="mb-0">
         <button class="btn btn-link collapsed" aria-expanded="true" aria-controls="generaldata"> 
            General data
         </button>
      </h5>
   </div>
   <div class="card-body collapse show" role="tabpanel" id="generaldata" aria-labelledby="generaldata-header">
   ...

In order to have the chance to style the header based on the expansion state, I recommend adding a class like in my PR.

@benouat
Copy link
Member

benouat commented Jul 3, 2018

Thank you for the explanation.
Unfortunately I stand on my position, I don't see any difference.

Let me try to explain:

Before, as you mentioned, we had an inline html element, the a directly inside the <div class="card-header" > on which you added the css code to have your chevron.

Now we have an extra block element, the <h5> that contains another inline element, the <button> which behave exactly the same (visually speaking) as the previous <a>. It is using btn-link class.

So you can continue to do the exact same thing you were doing...

.card-header button[aria-expanded=true]:after {
    font: 18px/1 FontAwesome;
    content: "\f078";
    margin-top: 0;
    float: right;
    color: #fff;
}

Previously, the <a> you were styling

image

Now, the <button> you should be styling

image


Here is a Stackblitz showing you can do it in the same way.
Hope it helps !

@pkozlowski-opensource
Copy link
Member

I tend to agree with @benouat analysis - the same work-around should be possible with new markup. Please let us know if something is still blocking so we can have a look.

In any case we want to introduce more flexibility for styling / marking up accordion title via existing issues (#465, #717).

@pvelez-bp3
Copy link

Trying to put a button or an anchor with a different action than toggle the content into the title(ngbPanelTitle) does not work in IE11 and firefox because of Content model for button: "Phrasing content, but there must be no interactive content descendant". Any clue on how to make this work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants