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

Modal component properties is not clearing when reusing the component #21

Closed
Nks opened this issue Oct 27, 2022 · 5 comments
Closed

Modal component properties is not clearing when reusing the component #21

Nks opened this issue Oct 27, 2022 · 5 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Nks
Copy link

Nks commented Oct 27, 2022

Version: 0.4.3

Problem

When defining the component, using it for different states, and passing the props to the component with different values, it is not using a new component.

How to reproduce

Define the modal one time and reuse it for different states:

useAlert.ts

import Alert from '~/components/Modals/Alert.vue';
import { createConfirmDialog } from 'vuejs-confirm-dialog';

const alertModal = createConfirmDialog(Alert);

export const showModal = async (options: Modal.Options) => {
    await alertModal.close()

    return alertModal.reveal(...[options]);
};

export const showDangerMessage = (options: Modal.Options) => {
    options.type = 'danger';

    return showModal(options);
};

export const showInfoMessage = (options: Modal.Options) => {
    options.type = 'info';

    return showModal(options);
};

export const showWarningMessage = (options: Modal.Options) => {
    options.type = 'warning';

    return showModal(options);
};

export const showSuccessMessage = (options: Modal.Options) => {
    options.type = 'success';

    return showModal(options);
};

alerts.vue

<script setup lang="ts">
import DefaultButton from "~/components/Buttons/DefaultButton.vue";
import {showDangerMessage, showInfoMessage, showWarningMessage} from "~/composables/useAlert";
import {definePageMeta} from "#imports";

definePageMeta({
    layout: 'dashboard'
})

const confirmDelete = async () => {
    const {data, isCanceled} = await showWarningMessage({
        title: 'Confirm Delete',
        message: 'Are you sure want to delete this object?',
        confirmButton: 'Confirm & Delete',
        cancelButton: 'Cancel this actions',
        dismissible: false
    })

    if (!isCanceled) {
        await showInfoMessage({title: 'You confirmed the action', message: 'There is nothing to do with anymore.'})
    } else {
        await showInfoMessage({title: 'You canceled this action', message: 'There is nothing to do with anymore.'})
    }
}
</script>

<template>
    <div>
        <DefaultButton
            class="mr-2"
            @click.prevent="showDangerMessage({title: 'Something went wrong', message: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'})">
            Show Error Message
        </DefaultButton>
        <DefaultButton
            class="mr-2"
            @click.prevent="showInfoMessage({title: 'Info Message', message: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'})">
            Show Info Message
        </DefaultButton>
        <DefaultButton
            class="mr-2"
            @click.prevent="showWarningMessage({title: 'Warning Message', message: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'})">
            Show Warning Message
        </DefaultButton>
        <DefaultButton
            class="mr-2"
            @click.prevent="showWarningMessage({title: 'Warning Message', message: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'})">
            Show Warning Message
        </DefaultButton>
        <DefaultButton @click.prevent="confirmDelete">
            Confirm Delete
        </DefaultButton>
    </div>
</template>

Alert.vue

<script setup lang="ts">
import DefaultButton from "~/components/Buttons/DefaultButton.vue";
import {onMounted} from "vue";
import DangerButton from "~/components/Buttons/DangerButton.vue";

interface Props {
    title: string;
    message: string;
    dismissible?: boolean,
    autoHide?: boolean;
    timeout?: number;
    confirmButton?: string;
    cancelButton?: string | boolean;
    type: Modal.Type
}

const props = withDefaults(defineProps<Props>(), {
    dismissible: true,
    autoHide: false,
    timeout: 10000,
    confirmButton: 'Close',
    cancelButton: false
})

const emit = defineEmits<{
    (event: 'confirm'): void
    (event: 'cancel'): void
}>()

onMounted(() => {
    if (props.autoHide) {
        setTimeout(() => {
            emit('cancel')
        }, props.timeout)
    }
})
</script>

<template>
    <Modal
        :model-value="true"
        :dismissible="dismissible"
        :class="`modal-type-${type}`"
        @close="emit('cancel')"
    >
        <div class="text-center text-[24px]">
            {{ title }}
        </div>
        <p class="text-center">{{ message }}</p>
        <div class="text-center pt-4 flex">
            <DangerButton v-if="cancelButton" class="flex-grow mr-2" @click="emit('cancel')">
                {{ cancelButton }}
            </DangerButton>

            <DefaultButton class="flex-grow" @click="emit('confirm')">
                {{ confirmButton }}
            </DefaultButton>
        </div>
    </Modal>
</template>
CleanShot.2022-10-27.at.18.04.46.mp4

Possible solution

When executing the reveal method, it should re-init the children component with new properties - in this case, it will clear the properties passed before.

Temporary solution

Init the component every time with const alertModal = createConfirmDialog(Alert);

@harmyderoman
Copy link
Owner

Hi, @Nks! Thank you for raising this important issue!
I thought it was intuitively understood that when you 'create dialog', you create it with its props( I think it's better to call it attributes in this case), so the dialog holds them and changes only attributes that need to change. So it is important to pass all data that you wanna change to reveal. In your case it will look like this:

showDangerMessage({title: 'Something went wrong', message: 'Lorem', cancelButton: false , autoHide: false})

I really don't wanna make breaking changes, and I want to help you. For the global solution, we can think about adding a new method to dialog instance clearAttrs, so it will behave as you expected. For example:

export const showModal = async (options: Modal.Options) => {
    await alertModal.close()
    alertModal.clearAttrs()

    return alertModal.reveal(...[options]);
};

Or we can make method setAttrs and clear attributes like this setAttrs({}).

@harmyderoman harmyderoman added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed labels Oct 28, 2022
@harmyderoman
Copy link
Owner

Ok, an alternative solution with backward compatibility. You can set keepClean options in createConfirmDialog, so it will solve this bug, but it will not break old code:

const propsBehaviorOptions =  { keepClean: true, keepInitialAttrs: true }
const dialog = createConfirmDialog(ModalDialog, { title: 'Title' }, propsBehaviorOptions ) 

keepInitialAttrs will set initial attrs on removing dialog. If false will clear all attrs, props will get default values from a component.

By default, I will set all options to false for backward compatibility.

@harmyderoman harmyderoman added the bug Something isn't working label Oct 28, 2022
@harmyderoman
Copy link
Owner

@Nks Fix is on the way: props-fix!

You can fix your issue by using it like that:

  const dialog = createConfirmDialog(Modal, 
  { message: 'initial message' }, 
  { chore: true, keepInitial: false })

Options are optional, so if you don't use them, the dialog's props will behave like previously.
Let me know what you think about this fix!

@harmyderoman harmyderoman removed the help wanted Extra attention is needed label Nov 10, 2022
@Nks
Copy link
Author

Nks commented Nov 17, 2022

Hey @harmyderoman.

Thank you for looking at this!

Yes, I think additional parameters should work fine. As I said - the temporary solution is to init the dialog whenever we want to show the dialog works too.

I'll check the suggested solution this week.

Thank you again!

@harmyderoman
Copy link
Owner

Fixed by #22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants