Views dialogs don't close via escape.

Close via Esc does work, but not on the second and third modals in a flow. I'm looking into this.

Confirm that close on Esc works (chrome, ff)
Maybe purpose of the issue is disable this?

dawehner, what dialog did you test? Can you point me to a path?

Reproduced, I think.

Everything works fine on FF, as noted.

On Chrome. the esc key closes the dialog if I simply open the Add field dialog and press esc. If I choose an option in the dialog and press apply, so that the form in the dialog refreshes, then the esc key no longer closes the dialog.

Does that describe your experience dawehner?

Ok, I know the "why" now. At first I though it had something to do with the keypress binding getting lost when the contents of the dialog refreshes. So I tested this theory and found that the handler fires on the first load of the dialog and after a refresh of the content in a multistep form. So this isn't the issue. BUT, I did notice that sometimes the esc key press wouldn't fire in Chrome if the dialog didn't have focus. So after clicking on it a couple times to give it focus, it finally dawned on me that this is the real issue.

In Firefox, some element of the dialog has focus as we move from the first form to the second in a multiform sequence. We know this by checking the document.activeElement property on the page.

Notice that in the first dialog load, the select box is in focus. On the second page in the form, the .ui-dialog element has the page focus.

In Chrome, we lose focus on the dialog after it refresh for the second page in the multiform. The focus shifts to the body.

So pressing the esc key never bubbles through the dialog element.

Now, I don't yet know why we're getting inconsistent focus behavior, but at least I know what to fix!

adding JavaScript tag.

So, the problem comes down to this little bit of code in jquery.ui.dialog.js in the open method:

if ( this._isOpen ) {
	if ( this._moveToTop() ) {
		this._focusTabbable();
	}
	return;
}

When we load content into the dialog via AJAX, we call the openDialog method in the Drupal dialog code. This in turn invokes the dialog method on the existing dialog wrapper. If the dialog is already open (this._isOpen), then the jQuery UI dialog simply checks to see if this dialog was moved -- that is, is there anything visible in the DOM after this modal (i.e. any following siblings before the end body tag)? If not, then it simply returns without calling the rest of the open method code. So, the this._focusTabbable method is not called when we refresh the content of the dialog via AJAX. Bummer :/

I'm looking for the best way to get around this without hacking jQuery UI, which is not really an option. Our dialog code should do something to refocus the first tabbable when we call our proprietary open method on an existing jQuery UI dialog.

The jQuery UI Dialog _moveToTop method is checking to see if anything in the DOM follows the modal dialog. If yes, then it has "moved" and focus is recalculated.

_moveToTop: function( event, silent ) {
	var moved = !!this.uiDialog.nextAll(":visible").insertBefore( this.uiDialog ).length;
	if ( moved && !silent ) {
		this._trigger( "focus", event );
	}
	return moved;
}

So we trick jQuery UI into thinking that something exists after an already-open dialog.

function openDialog (settings) {
  settings = $.extend({}, drupalSettings.dialog, options, settings);
  // Trigger a global event to allow scripts to bind events to the dialog.
  $(window).trigger('dialog:beforecreate', [dialog, $element, settings]);
  // If a modal dialog is already open and its contents are simply being
  // replaced, then we must force the jQuery UI dialog to find a tabbable
  // element within the new content. We do this by inserting a blank div
  // after the modal, tricking jQuery UI into thinking that the modal was
  // moved. A moved modal will have its focus reset. The empty div is then
  // removed from the DOM.
  // @see https://drupal.org/node/2113567
  var $div = $();
  if ($element.data('ui-dialog')) {
    $div = $('<div></div>').insertAfter($element.dialog('widget'));
  }
  $element.dialog(settings);
  $div.remove();
  dialog.open = true;
  $(window).trigger('dialog:aftercreate', [dialog, $element, settings]);
}

Why trick jQuery? Well, I want to have the stand focus calculation run, rather than just setting the focus ourselves, because the dialog widget has a focus callback that gets triggered when the focus is recalculated. By tricking it, we ensure that this callback is invoked appropriately when we refresh the content of an existing dialog.

I agree, great work on finding out and spelling out the problem. The solution is simpler though.

The focus problem is when content is changed through AJAX right? so the focus issue should be solved in a behavior. See the patch, we're even removing a sort of hack as well and no tricking of jQuery needed.

Confirmed that modals don't close using esc without this patch.

Tested patch in Chrome and FF with overlay by opening admin/structure/views/view/content, adding two new fields and then attempting to use esc to close the modal. This worked fine for me.

@dawehner dialog.ajax.js is properly added, check the network tab and filter by xhr requests, it should show up whenever you open a modal in views.

If we just call focus on the dialog element, then the dialogfocus event will only be triggered when the dialog opens, not on subsequent focus events. Try out this code in dialog.js for an example of the focus callback. It will only be invoked when the Views dialog is opened, not on subsequent content refresh/refocus. I think we do need to trick jQuery UI in order to maintain consistent behavior.

drupalSettings.dialog = {
  autoOpen: true,
  dialogClass: '',
  // When using this API directly (when generating dialogs on the client side),
  // you may want to override this method and do
  // @code
  // jQuery(event.target).remove()
  // @endcode
  // as well, to remove the dialog on closing.
  close: function (event) {
    Drupal.detachBehaviors(event.target, null, 'unload');
  },
  focus: function (event) {
    console.log(event.type);
  }
};

nod_ and I had a chat and he's convinced me that the approach in #14 is fine. Specifically:

so when you change the content of a modal it's not like you're clicking outside and back inside the modal

I agree. So, let's go with the patch in #14.

So is this our bug really, or is it worth filing an upstream report for jQuery UI?

Committed/pushed to 8.x, nice work tracking this down, leaving open in case it's worth opening the bug report, can close if it isn't, or if we open one :)

I don't feel it's an upstream issue. We're replacing the content with ajax commands and running behaviors on top of that. Not typical jQuery UI stuff :)

Also I'm not even sure it's possible to fix upstream.