Improve the hide-show logic of ULS

* The events are suppressed on click of ULS trigger, it is an anti
  pattern. It can cause other overlay dialogs, if any, to stay with
  ULS and causes UI glitch. This patch just propagates the events and not
  eats up.
* The show method was hiding all other ULS dialoges open using a
  global $('.uls-menu').hide(). This is again not a good pattern.
  A plugin instance should not interfere with other instance's state.
  More over, calling jQuery hide() method on menu instead of plugin's
  hide method leaves the other plugin instance in a corrupted state.
  The plugin hide method does more things than just hiding the menu.
  It has a 'shown' book keeping property to update. This kind of
  corrupted state was causing bugs like https://phabricator.wikimedia.org/T114123
* While avoiding the above two antipatterns, the way ULS was hidden when
  clicked on any 'other' part of body was improved. It now uses event.target
  to correctly handle the 'click-outside-hide' logic

All these above changes does not change any existing UX.

Change-Id: I40b355115cbda54a68e8d58d3750fb9f1c3b6920
This commit is contained in:
Santhosh Thottingal
2015-09-30 10:54:04 +05:30
parent 423b533b70
commit d4de09ffb9

View File

@@ -177,9 +177,6 @@
this.initialized = true;
}
// hide any other visible ULS
$( '.uls-menu' ).hide();
this.$menu.show();
this.$menu.scrollIntoView();
this.shown = true;
@@ -321,7 +318,11 @@
/**
* On cancel handler for the uls menu
*/
cancel: function () {
cancel: function ( e ) {
if ( e && this.$element.is( e.target ) ) {
return;
}
this.hide();
if ( this.options.onCancel ) {
@@ -353,10 +354,7 @@
}
},
click: function ( e ) {
e.stopPropagation();
e.preventDefault();
click: function () {
if ( this.shown ) {
this.hide();
} else {