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

Add card Bootstrap tabs navigation #119

Merged

Conversation

cavasinf
Copy link
Collaborator

@cavasinf cavasinf commented Oct 3, 2022

Description

As discussed in #116

TODO 📋

  • Bootstrap tab Navigation
  • Url link navigation
  • Validate complex HTML variable (set, include, macro)
  • Documentation

Bootstrap tab navigation

All content tab must be rendered

{% set items = [
    {
        name: 'Account',
        content: '<h1>My Account</h1><h3>Profile Details</h3>'
    },
    {
        name: 'Notifications',
        content: include('notifications.html.twig')
    },
    {
        name: 'Experience',
        header: true
    },
    {
        name: 'Feedback',
        content: '<h1>Give Feedback</h1>'
    },
    {
        name: 'Automates',
        header: true
    },
    {
        id: 'auto',
        name: tabler_icon('robot') ~ " " ~ 'label.variable.automated.plural'|trans|capitalize,
        raw: true,
        content : _self.automate_content(mentions, automated_values)
    }
] %}

{% embed '@Tabler/embeds/card-vertical-navigation.thml.twig' with {items : items} %}{% endembed %}

card nav

Url single content navigation

Only one tab content is completed, rest is only for navigation between urls

{% set items = [
    {
        name: 'Account',
        url: '/account'
    },
    {
        name: 'Notifications',
        url: '/notifications',
        active: true,
    },
    {
        name: 'Experience',
        header: true
    },
    {
        name: 'Feedback',
    },
] %}

{% embed '@Tabler/embeds/card-vertical-navigation.thml.twig' with {items : items} %}
    {% block content %}
        <h1>Notifications</h1>

        <p>Here's my content manually inserted!</p>
    {% endblock %}
{% endembed %}

image

Only nav card part (with empty content)

{% set items = [
    {
        name: 'Account',
        url: '/account'
    },
    {
        name: 'Notifications',
        url: '/notifications',
        active: true,
    },
    {
        name: 'Experience',
        header: true
    },
    {
        name: 'Feedback',
    },
] %}



<div class="row">
    <div class="col-3">
        {% embed '@Tabler/embeds/card-vertical-navigation.thml.twig' with {items : items} %}
            {% block nav_size %}col{% endblock %}
            {% block nav_border %}{% endblock %}
            {% block content_display %}d-none{% endblock %}
            {% block content %}{% endblock %}
        {% endembed %}
    </div>
</div>

image

Empty include

    {% include '@Tabler/embeds/card-vertical-navigation.thml.twig'%}

image

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

@cavasinf cavasinf added Status: Needs Review Not under investigation Feature Feature requested labels Oct 3, 2022
@cavasinf cavasinf added this to the 1.1 milestone Oct 3, 2022
@cavasinf
Copy link
Collaborator Author

cavasinf commented Oct 3, 2022

So.. first try to do it with complex HTML/Twig in params..

To complete content I used macro or include, because placement of the code has no importance.
Instead of {% set xxx %}{% endset %} must be BEFORE the embed code.
It's more clear for me to have the embed first and details under the embed.

Macro detailed

{% macro automate_content(mentions, automated_values) %}
    <h3>{{ tabler_icon('robot') }} {{ 'label.variable.automated.plural'|trans }}</h3>

    {% embed '@Tabler/embeds/card.html.twig' with {fullsize : true} %}
        {% from '@Tabler/components/button.html.twig' import button %}
        {% block box_body %}
            <div class="table-responsive">
                <table class="table table-vcenter card-table">
                    <thead>
                    <tr>
                        <th scope="col">{{ 'label.variable'|trans }}</th>
                        <th scope="col">{{ 'label.example'|trans }}</th>
                        <th class="w-1" scope="col"></th>
                    </tr>
                    </thead>
                    <tbody>
                    {# @var automatedMention \App\Structure\QuillMentionItem #}
                    {% for name, automatedMention in mentions.automated %}
                        <tr {{ stimulus_controller('clipboard') }}
                                data-clipboard-success-content='{{ tabler_icon('success') }}'>
                            <td>
                                <code {{ stimulus_target('clipboard','source') }}>{{ automatedMention }}</code>
                            </td>
                            <td>{{ automated_values[automatedMention.value] }}</td>
                            <td class="text-right">
                                <div class="btn-list flex-nowrap">
                                    {{ button('copy', {
                                        title : 'action.copy'|trans,
                                        buttonType : 'button',
                                        attr : {
                                            'data-clipboard-target' : 'button',
                                            'data-action' : "clipboard#copy"
                                        }
                                    }, 'primary') }}
                                </div>
                            </td>
                        </tr>
                    {% else %}
                        <tr>
                            <td colspan="999">{{ 'label.no_data_found'|trans }}</td>
                        </tr>
                    {% endfor %}
                    </tbody>
                </table>
            </div>
        {% endblock %}
    {% endembed %}
{% endmacro %}

@cavasinf
Copy link
Collaborator Author

Any words on that @kevinpapst?

@cavasinf cavasinf marked this pull request as ready for review October 10, 2022 14:17
@kevinpapst
Copy link
Owner

kevinpapst commented Oct 10, 2022

I don't have the time right now, sorry! Will get back to you asap.

@@ -8,3 +8,6 @@
{% endif %}
{% endmacro %}

{% macro uniqueId() -%}
{{ 'now'|date('YmdHisu') }}
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please create a twig function tabler_unique_id() for that, the current approach looks dangerous.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the beginning, I was about to add a twig function, but I thought it was overpower for a template Bundle. But I'm good with that!

approach looks dangerous

Why ? I'll add uniqueId() PHP function that do the same as the current twig function : Gets a prefixed unique identifier based on the current time in microseconds.
Just tell me why you find that dangerous not why I should do it (I'll do it anyway 😉)

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, I didn't even know until now, that that uniquid() relies on microseconds as well.
Which makes my comment kinda invalid on that end 😁
We still might need it somewhere else later on, so having it available doesn't hurt.

Actually I remember having problems with duplicate IDs in twig (or more correct in the javascript part) and therefor moved an identical method to a twig function. That allowed me to use a class member of the twig extension to increment on every call. That counter was then appended to the generated ID, so even in the same microsecond you received a unique ID.

Maybe I am getting old and that is not true (anymore), but somewhere in my head I have stored the information, that microseconds are not reliable across all systems and should be avoided when it comes to generating random data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am getting old

😆😆😆😆

Funny but I have the same thought on default UUID value from SQL.
Same as uniqId it is generated by microseconds and others.. I was frustrated than generating multiple UUID from a big INSERT will create duplicates.

ATE, microseconds ids are 99% non reproducible, unless you change the server time 👀

Copy link
Owner

Choose a reason for hiding this comment

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

Oh, good to know.
I thought UUID is truly unique ... shit, I am using it in one project as well 😨
The server time-change is happening twice a year during summer-winter shift, so that is really dangerous for database (not twig though).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You have 1/10 000 000 chance to generate the same UUID V1 for ONE same second.
https://en.wikipedia.org/wiki/Universally_unique_identifier#Versions

More chance than winning the EuroMillion, but still slight 😉

Copy link
Owner

Choose a reason for hiding this comment

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

Ah well then, I guess I am fine to live with that chance

@kevinpapst
Copy link
Owner

@wucherpfennig do you probably have a use-case and could test here?

@cavasinf I have no use for such a component currently and I am coffein-deficient, so I am struggeling with building a demo 😁 but I am okay to merge it after the unique id change!

@wucherpfennig
Copy link
Contributor

I appreciate the new feature very much!

and I will have some product preview feature where I can test it. But currently I'm in refactoring hell with some business code...

The suggested changes look good / 😃👍 to me.

wucherpfennig
wucherpfennig previously approved these changes Oct 15, 2022
Copy link
Contributor

@wucherpfennig wucherpfennig left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@cavasinf cavasinf added Status: Reviewed Has staff reply/investigation and removed Status: Needs Review Not under investigation labels Oct 17, 2022
@kevinpapst
Copy link
Owner

Can you run codestyle fixer please

kevinpapst
kevinpapst previously approved these changes Oct 17, 2022
@kevinpapst
Copy link
Owner

Ups, approved before I saw the failed test.
Can you please add the new function name to the test and I will merge.

@kevinpapst kevinpapst merged commit 701da90 into kevinpapst:main Oct 17, 2022
@cavasinf
Copy link
Collaborator Author

Done 👍

@kevinpapst kevinpapst modified the milestones: 1.1, 1.0 Oct 17, 2022
@cavasinf cavasinf deleted the feature/card-vertical-navigation branch October 17, 2022 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Feature requested Status: Reviewed Has staff reply/investigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants