bastinald / laravel-livewire-modals

Dynamic Laravel Livewire Bootstrap modals.
66 stars 31 forks source link

JS & Crud improvements: protect and singleton #3

Closed Thiktak closed 2 years ago

Thiktak commented 3 years ago

Hello,

I use this modal component, and I have some possible improvements:

modal.js > use getOrCreateInstance instad of new Modal.

This will prevent to have several overlay displayed (without possibility to clean them), if we have modal with modal link inside or if we want to do some navigation.

Livewire.on('showBootstrapModal', () => {
    let modal = Modal.getOrCreateInstance(modalsElement);
    if( modal ) {
        modal.show();
    }
});

modal.js > protect modal.hide

If we trigger event hideModal, and this one does not exists ... there is an error. Linked to next improvement too.

Livewire.on('hideModal', () => {
    let modal = Modal.getInstance(modalsElement);
    if( modal ) {
        modal.hide();
    }
});

Crud: reverse the two emits

Currently we have $this->emit('$refresh'); $this->emit('hideModal'); but I think it could be better to have:

        $this->emit('$refresh');
        $this->emit('hideModal');

Indeed, this will prevent the system to not refresh (i.e.: the list), if the modal does not exists (i.e.: trigger event via another Component) and/or protect the modal.hide().

Crud: modal-body

I created a system to change the modal in "columns", and I had some issues when I tried to play with the layout & css (flex, overflow). To provide more flexibility, could it be possible to separate the modal-body and the content ?

        <div class="modal-body">
            <div class="d-grid gap-3"></div>
        </div>

If everything is on the same tag, the flex propriety of the d-grid is filling all the height of the modal-body, and the inputs are like that : 2021-07-03 18_41_42-Window

bastinald commented 2 years ago

stacking modals like this creates too much complexity for this package to handle

not sure why you need to do this