Skip to content

Commit

Permalink
Fixed closing of open dropdown.
Browse files Browse the repository at this point in the history 
Fixes
soundasleep/jquery-dropdown#127 (comment)
by merging https://github.com/eoghanmurray 's solution
(soundasleep/jquery-dropdown@4db65fc).
  • Loading branch information
@mnlipp
mnlipp committed Aug 29, 2017
1 parent 2e9f4c9 commit 50d336b
Showing 1 changed file with 14 additions and 9 deletions.
23 changes: 14 additions & 9 deletions org.jgrapes.portal/resources/org/jgrapes/portal/lib/jquery-dropdown-2.0.3/jquery.dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,35 @@ if (jQuery) (function ($) {
// In some cases we don't hide them
var targetGroup = event ? $(event.target).parents().addBack() : null;

// Things to hide, by default every dropdown visible
var toHide = $(document).find('.jq-dropdown:visible');

// Are we clicking anywhere in a jq-dropdown?
if (targetGroup && targetGroup.is('.jq-dropdown')) {
// Is it a jq-dropdown menu?
if (targetGroup.is('.jq-dropdown-menu')) {
// Did we click on an option? If so close it.
if (!targetGroup.is('A')) return;
} else {
// Nope, it's a panel. Leave it open.
return;
}
// Nope, it's a panel. Leave it open, but close others
toHide = $(document).find('.jq-dropdown:visible').not(
$(event.target).parents('.jq-dropdown')); }
}

// Hide any jq-dropdown that may be showing
$(document).find('.jq-dropdown:visible').each(function () {
toHide.each(function () {
var jqDropdown = $(this);
// Remove all jq-dropdown-open classes from things to hide...
jqDropdown.find('.jq-dropdown-open').removeClass('jq-dropdown-open');
// ... and from whatever caused it to be shown
$(document).find('[data-jq-dropdown=\'#' + this.id + '\']').
removeClass('jq-dropdown-open');
// Hide and trigger
jqDropdown
.hide()
.removeData('jq-dropdown-trigger')
.trigger('hide', { jqDropdown: jqDropdown });
});

// Remove all jq-dropdown-open classes
$(document).find('.jq-dropdown-open').removeClass('jq-dropdown-open');

}

function position() {
Expand Down

6 comments on commit 50d336b

@eoghanmurray
Copy link

@eoghanmurray eoghanmurray commented on 50d336b Aug 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

From a look at the changes, I believe that this version has the drawback that if you have a dropdown-menu within a dropdown-panel, and you select a menu item from the nested menu, then both the nested menu and it's parent panel will be closed, which I don't think is what you want — haven't tested though.

@eoghanmurray
Copy link

Choose a reason for hiding this comment

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

The problem with e.g. targetGroup.is in this context is that it doesn't differentiate between the current dropdown and parent dropdowns; targetGroup.is('.jq-dropdown-menu') will return true if any parents are a menu.

@mnlipp
Copy link
Owner Author

@mnlipp mnlipp commented on 50d336b Aug 30, 2017

Choose a reason for hiding this comment

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

... that if you have a dropdown-menu within a dropdown-panel, and you select a menu item from the nested menu, then both the nested menu and it's parent panel will be closed, which I don't think is what you want

As it happens, it is what I want. I have a "Settings" icon that opens a dropdown-panel which shows the various kinds of settings available (a "settings page" or "settings overview"). Some settings are then changed by selecting a new value from a dropdown menu.

Now, in my case, I assume the common use case to be the change of a single setting, so it is convenient for the user that all drop downs close after the selection has been made. You could say that for me a dropdown panel is just like a menu in a nested menu structure, except that the items in that particular menu cannot easily be represented as a list and therefore need the freedom that a panel offers.

Of course, it would be a valid alternative to consider the opening of the "settings (dropdown) panel" the start of a customization process that involves changing several values and requires an explicit close (e.g. by clicking outside the panel -- though I'd offer a close icon/link in addition).

@eoghanmurray
Copy link

@eoghanmurray eoghanmurray commented on 50d336b Aug 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That's fine for your usecase and this repository.
Would you agree though that for the general jquery-dropdown project, the other behaviour makes more sense? I can imagine a configuration popup which has a nested dropdown for each setting and a final 'save' button would be a much more common case.
(apologies for using overly personal 'my' and 'you' in the following)
With my behaviour, it's possible to listen for the 'hide' event on the nested dropdown and auto close the parent but with your behaviour, I imagine (although I haven't tried) that it's much more difficult to prevent the auto closing of the parent.

@mnlipp
Copy link
Owner Author

@mnlipp mnlipp commented on 50d336b Aug 31, 2017

Choose a reason for hiding this comment

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

Not having actual use case data (except from the two of us) makes this a bit philosophical. And, of course, as nobody has officially "adopted" the project, we're all free to follow our own agenda. If I was a bit more of a jquery expert, I would have started a fork, but I'm still trying to fully understand the architecture and therefore I don't feel qualified for this.

This being said, I don't agree with you that a dropdown panel should require an explicit close as default behavior. To me, the dropdown panel and the dropdown menu are just two possibilities to "style" whatever options you want to offer to the user. Nested dropdowns represent a decision tree, no matter what they are composed of. Once the decision is made, that's it.

If I was to develop this library further, I'd introduce the "dropdown dialog" as a flavor of the dropdown panel to cover your use case. A dialog is known to stay open until it is explicitly closed. If I was to do things properly, I'd also make an attempt to implement this "dropdown dialog" as a "merge" of "dropdown panel" and the "standard" jquery-ui dialog, i.e. make its usage as similar to the existing dialog as reasonably possible.

@eoghanmurray
Copy link

@eoghanmurray eoghanmurray commented on 50d336b Aug 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I hear you — I'm thinking of the dropdown-menu as being like a replacement for a <select>, whereas the dropdown-panel is something more; something that can contain many elements and configuration details and which you explicitly dismiss. I've got a <form> and a submit/save button in my panel.

Nested dropdowns represent a decision tree, no matter what they are composed of.

My <form> within a panel is a single counter example showing that there are many more usecases than just a decision tree.

Please sign in to comment.