django-crispy-forms / crispy-bootstrap5

Bootstrap5 template pack for django-crispy-forms
MIT License
462 stars 76 forks source link

Add missing css_class to accordion template. #169

Closed ghost closed 5 months ago

ghost commented 9 months ago

I've added missing attribute to accordion template and written additional test.

smithdc1 commented 9 months ago

Hi @rockenchanter

The linked issue says

css_class attribute does not work with Accordion and AccordionGroup

This PR proposes changes the Accordion template. What's your thoughts about AccordionGroup?

ghost commented 9 months ago

With AccordionGroup it's a little more complicated, initially I tried to apply the same changes to accordion-group.html like this:

<div class="accordion-item{% if div.css_class %} {{ div.css_class }}{% endif %}">
    <h2 class="accordion-header">
        <button class="accordion-button{% if not div.active %} collapsed{% endif %}" type="button" data-bs-toggle="collapse" data-bs-target="#{{ div.css_id }}" aria-expanded="true"
            aria-controls="{{ div.css_id }}">
            {{ div.name }}
        </button>
    </h2>

    <div id="{{ div.css_id }}" class="accordion-collapse collapse{% if div.active %} show{% endif %}"
         aria-labelledby="{{ div.css_id }}" {% if not div.always_open %} data-bs-parent="#{{ div.data_parent }}" {% endif %}>
        <div class="accordion-body">
            {{ fields|safe }}
        </div>
    </div>  
</div>

But it causes few test failures because Accordion's render method sets active attribute to True on first AccordionGroup, which in turn appends active class to css_class attribute inside Container's render method, all of this renders:

  <div class="accordion-item active"> <!-- active class is appended here -->
    <h2 class="accordion-header">
      <button class="accordion-button" type="button" data-bs-toggle="collapse" data-bs-target="#one"
        aria-expanded="true" aria-controls="one">
        one
      </button>
    </h2>
    <div id="one" class="accordion-collapse collapse show" aria-labelledby="one" data-bs-parent="#accordion-7311">
      <div class="accordion-body">
        <div id="div_id_first_name" class="mb-3">
          <label for="id_first_name" class="form-label requiredField">
            first name<span class="asteriskField">*</span>
          </label>
          <input type="text" name="first_name" maxlength="5" class="textinput textInput inputtext form-control" required
            id="id_first_name" />
        </div>
      </div>
    </div>
  </div>

Templates accordion.html, accordion_always_open.html and accordion_flush.html inside tests/results folder obviously do not have active class which is the cause of test failures. So now I'm wondering what solution would apply nicely here.