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

Close popup when clicking next to it #1053

Closed
wants to merge 2 commits into from
Closed

Conversation

AIC-BV
Copy link
Contributor

@AIC-BV AIC-BV commented Feb 9, 2024

Close popup when clicking next to it

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 9, 2024

Can probably be optimised by reusing the right function but I couldn't find how to call it without having undefined error

@LukeTowers
Copy link
Member

@AIC-BV would you be able to provide a screen recording of the new behaviour?

@bennothommo
Copy link
Member

@AIC-BV does this functionality make it if you click on the translucent overlay, it closes the modal? I'm not a big fan of that at all - too many times, people mis-click and miss something important (or lose a completed form if the modal contains one).

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 21, 2024

@AIC-BV does this functionality make it if you click on the translucent overlay, it closes the modal? I'm not a big fan of that at all - too many times, people mis-click and miss something important (or lose a completed form if the modal contains one).

That is the case! For me it is good UX

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 21, 2024

@LukeTowers

Bewerk.bestelling._.AIC.BV.-.Google.Chrome.2024-02-21.09-17-56.mp4

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 22, 2024

It is a read only popup relation and it is very annoying if you can't click next to it 😆

@bennothommo
Copy link
Member

@AIC-BV

That is the case! For me it is good UX

Apologies, but I strongly disagree.

I would begrudgingly allow it for "message" modals, where an action has already occurred and the modal is simply letting the user know it's done - albeit, a flash message would generally be better in this case and those can already be easily dismissed. Definitely not for any modal that requires user interaction or attention, ie. a modal form, a confirmation modal where the user has to Yes or No, or an error message.

I would only accept this PR if such modal behaviour were optional (ie. could be defined by configuration) and by default, it was disabled.

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 22, 2024

@bennothommo
If acceptable I can adjust #1044 to add variable "closeModalWhenClickingNextToIt". Could do with a better name.

And then update this PR to

if (this.options.closeModalWhenClickingNextToIt) {
    modal.on('click', function(e) {
        const target = e.target;
        if (target.classList.contains('control-popup')) {
            modal.hide()
            $('.popup-backdrop').remove()
            $(document.body).removeClass('modal-open')
        }
    });
}

Let me know

@bennothommo
Copy link
Member

That'd be all good. Maybe allowDismiss as the option? Or closeOutside?

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Feb 23, 2024

Added in this PR
#1044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants