Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#9643 closed feature (wontfix)

modal dialog close on overlay click

Reported by: jurchiks Owned by:
Priority: minor Milestone: none
Component: ui.dialog Version: 1.10.3
Keywords: Cc:
Blocked by: Blocking:

Description

I had need for this functionality and found out that jQuery UI does not provide it built-in. I found it baffling because this is so simple to make (as I did), and googling for existing results yielded tons of discussions about this same feature, but, unfortunately, none of them worked for me, not the way I wanted them to.

Here's the patch: https://gist.github.com/jurchiks/7264596 I don't yet dare to try making a pull request as I've never worked with git in a multi-user environment, besides I also wanted to ask a question about the event I'm adding: perhaps it would be better to bind only the click event if the the option to close on click is enabled? I would think it's pointless to bind the mousedown event in that scenario, but perhaps not...

Change History (17)

comment:1 Changed 9 years ago by jurchiks

This is my original stackoverflow question with my initial attempts to solve the problem: http://stackoverflow.com/questions/19690131/jquery-ui-modal-dialog-global-close-on-overlay-click

comment:2 Changed 9 years ago by Scott González

Resolution: wontfix
Status: newclosed

I found it baffling because this is so simple to make

This is exactly why we shouldn't provide it. The goal is to make things easy, not to make everything one line of code.

On a side note, if clicking in random places can dismiss the dialog, the content probably isn't worthy of being modal.

comment:3 Changed 9 years ago by jurchiks

On a side note, if clicking in random places can dismiss the dialog, the content probably isn't worthy of being modal.

It's not random places, it's the overlay, besides, that's what the option is for, I didn't hardcode it. Google "jquery ui dialog close on overlay click" and see how many people have been looking for a solution to this, and how many have actually made something proper. Not providing it and just saying "it's easy, make it yourself" is an utterly ridiculous answer.

Last edited 9 years ago by jurchiks (previous) (diff)

comment:4 Changed 9 years ago by jurchiks

Why did you close the topic without even waiting for someone else's feedback on this? Now nobody will see it and it will be forgotten just because you think your oppinion is the only one that matters.

comment:5 Changed 9 years ago by Scott González

It's not random places, it's the overlay

The overlay is literally the entire page besides the one part of the page that is deemed the only thing important (that's what modal means). So, yes, it is random places.

Not providing it and just saying "it's easy, make it yourself" is an utterly ridiculous answer.

Actually, it's policy for all jQuery projects. If 80% of users don't need it or it can be implemented easily, then there is no need for it to be implemented by the team.

Why did you close the topic without even waiting for someone else's feedback on this?

Because I don't need anyone else's feedback. I'm aware of current and past discussions about features for the past several years.

Now nobody will see it and it will be forgotten just because you think your oppinion is the only one that matters.

My opinion is that of the team. Everyone that needs to see it will see it, regardless of the status. Do you think the team has never thought about this functionality before?

comment:6 Changed 9 years ago by jurchiks

Why make users come up with various hacks instead of implementing it in a proper way if it takes less than 10 lines of code to do it?

comment:7 Changed 9 years ago by jurchiks

Didn't expect this project to be THAT narrow-minded...

comment:8 Changed 9 years ago by Scott González

Why make users come up with various hacks instead of implementing it in a proper way if it takes less than 10 lines of code to do it?

Because 10 lines times 20 features = 200 lines. It also means 20 more options to document, making it near impossible to learn how to use the widget properly. It also means thousands of configuration combinations that need to work together. Implementation size for a feature is just one of many considerations.

Also, let's be very clear that we're not making users come up with various hacks. This is super straight forward to implement, and it's extremely easy for anyone to create a plugin which adds the option and put it on our hosted plugins site.

comment:9 Changed 9 years ago by jurchiks

making it near impossible to learn how to use the widget properly

Only if you want to use all the available options. Tell me one person who does that.

It also means thousands of configuration combinations that need to work together

Not all of them are not related.

This is super straight forward to implement

Took me two days to find this, and I had to dig inside jQuery UI dialog code to find what I was looking for, after I had tried every other suggested option. Others stopped at the first dirty hack that worked for them.

If 80% of users don't need it

To me it sounds like you're making the bare minimum expected from something like this and leaving everything else to the user, which, to me, is unacceptable for such a popular plugin. But who am I to speak of such things...

it's extremely easy for anyone to create a plugin which adds the option

If so, could you please make that plugin for me? I don't want to modify jQuery UI every time I update it, and my current solution (the one from the fiddle posted on the stackoverflow question) is IMHO a dirty hack, because the _createOverlay() function should be private and not overridable from outside.

comment:10 Changed 9 years ago by Scott González

Only if you want to use all the available options. Tell me one person who does that.

This is not true. You have to read the full documentation to learn how to use something properly. Just look at the mess that is jQuery.ajax(). There are tons of options. Nobody uses all of them. Tons of people get confused by the sheer insanity of the API. We have the same problem with the current draggable API.

If so, could you please make that plugin for me?

No. I think it's a terrible idea. Did you not catch on to that yet?

comment:11 Changed 9 years ago by jurchiks

No. I think it's a terrible idea. Did you not catch on to that yet?

Sorry, what exactly is a terrible idea? I don't know how to make a cleaner implementation of the functionality I want, you're saying you can do that by making a plugin, so show me then. You just said it's extremely easy, was that a lie?

Last edited 9 years ago by jurchiks (previous) (diff)

comment:12 in reply to:  11 Changed 9 years ago by tj.vantoll

Replying to jurchiks:

No. I think it's a terrible idea. Did you not catch on to that yet?

Sorry, what exactly is a terrible idea? I don't know how to make a cleaner implementation of the functionality I want, you're saying you can do that by making a plugin, so show me then. You just said it's extremely easy, was that a lie?

Even though this is not the place for this, I'll provide an example just to help promote how easy extending widgets is - http://jsfiddle.net/tj_vantoll/zNxgK/. However, please take any future conversations on this to the forums or Stack Overflow.

comment:13 Changed 9 years ago by jurchiks

But that is still a dirty hack because you're overriding a private method... Also, why are you using this.overlay.on(event, function(nested selector)) instead of "this._on(this.overlay, { click: 'close' });"? I don't see how it is better.

Last edited 9 years ago by jurchiks (previous) (diff)

comment:14 in reply to:  13 Changed 9 years ago by tj.vantoll

Replying to jurchiks:

But that is still a dirty hack because you're overriding a private method... Also, why are you using this.overlay.on(event, function(nested selector)) instead of "this._on(this.overlay, { click: 'close' });"? I don't see how it is better.

You're correct that _on() is the better approach. _createOverlay() is a publicly exposed extension point; we encourage such extensions. We're working on improving our documentation for these.

comment:15 Changed 9 years ago by jurchiks

But the underscore denotes a private variable/method. Private methods should never be overriden (and, technically, shouldn't be possible to be overriden).

comment:16 in reply to:  15 Changed 9 years ago by tj.vantoll

Replying to jurchiks:

But the underscore denotes a private variable/method. Private methods should never be overriden (and, technically, shouldn't be possible to be overriden).

For widgets the underscore just means the method cannot be invoked through the plugin. As I said we're working on improving our documentation for this. As an example see http://api.jqueryui.com/dialog/#extension-points.

comment:17 Changed 9 years ago by jurchiks

Right... Well, anyway, I really don't see the reason why something so simple as this mod wouldn't be committed; one example of its usage would be for popups that simply display some content without interaction (i.e. no buttons or forms inside the popup, just data). I also don't see how this would conflict with anything else because this would only work if modal option is enabled, which narrows its "area of effect" quite a lot.

Note: See TracTickets for help on using tickets.