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

added combined card-accordion component #165

Merged
merged 2 commits into from
Jun 6, 2023
Merged

added combined card-accordion component #165

merged 2 commits into from
Jun 6, 2023

Conversation

kevinpapst
Copy link
Owner

Description

Replaces #162

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I updated the documentation (see here)
  • I agree that this code will be published under the MIT license

@kevinpapst kevinpapst requested a review from cavasinf May 31, 2023 21:32
templates/embeds/collapsible.html.twig Outdated Show resolved Hide resolved
templates/embeds/collapsible.html.twig Show resolved Hide resolved
Comment on lines 6 to 8
{% set _id = id ?? tabler_unique_id('collapsible_') %}
{% set _open = open ?? false %}
{% set _id_collapse = _id ~ '_item' %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put that on top?
(part of me want that 😄)

Comment on lines 10 to 26
<div id="{{ _id }}" class="accordion accordion-flush{% block class %} {% endblock %}">
<div class="accordion-item {% block item_class %} {% endblock %}">
<div class="accordion-header" id="{{ _id }}_heading">
<button class="accordion-button {% if not _open %}collapsed{% endif %}"
type="button" data-bs-toggle="collapse" data-bs-target="#{{ _id_collapse }}"
aria-expanded="true">
{% block title %}{% endblock %}
</button>
</div>

<div id="{{ _id_collapse }}" class="accordion-collapse collapse {% if _open %}show{% endif %}">
<div class="accordion-body {% block body_class %}pt-0{% endblock %}">
{% block body %}{% endblock %}
</div>
</div>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that the accordion component is scary and not easy to manipulate/use.
But isn't it more "Maintainers" friendly to use it?
The less duplicate code we have to maintain, the happier we are?

@cavasinf
Copy link
Collaborator

cavasinf commented Jun 1, 2023

Still using blocks title and body.
I'll go like that:

{% set fullsize = true %}
{% extends '@Tabler/embeds/card.html.twig' %}
{% from '@Tabler/components/accordion.html.twig' import accordion %}

{% block box_class %}border-0{% endblock %}
{% block box_title %}{% endblock %}

{% set _accordionItem = {
    title: block('title') ?? '',
    body: block('body') ?? '',
    bodyClasses: '',
    options: {
        open: false,
        bodyExtraClass: 'border-top',
    }
} | merge(accordionItem ?? {}) %}

{% set _accordionOption = {} | merge(accordionOption ?? {}) %}

{% block box_body %}
    {{ accordion(
        [_accordionItem],
        _accordionOption
    ) }}
{% endblock %}

Used like that:

{% set item = {
    options : {
        open : true
    }
} %}
{% embed '@theme/embeds/collapsible.html.twig' with {accordionItem  : item} %}
    {% block title %}File permissions{% endblock %}
    {% block body %}
        .....
    {% endblock %}
{% endembed %}

Inside the doc, for accordionItem and accordionOption,
we can redirect users to the accordion doc at the specific object.

WDYT @kevinpapst?

@kevinpapst
Copy link
Owner Author

I'll give this a try later today ... full day of meetings.
Looks like a clever solution indeed, better than mine 😁

@kevinpapst
Copy link
Owner Author

kevinpapst commented Jun 1, 2023

Small adjustments applied, to simplify passing options.

    {% embed '@theme/embeds/collapsible.html.twig' with { boxtype: 'danger', border: false, open: true, flush: true } %}
        {% block title %}Foo{% endblock %}
        {% block body %}Bar{% endblock %}
    {% endembed %}

Can be reviewed @cavasinf

@kevinpapst kevinpapst merged commit 64a2919 into main Jun 6, 2023
3 checks passed
@kevinpapst kevinpapst deleted the collapsible branch June 6, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants