Convert Views to use the abstracted dialog modal

Working on it.

So i think the way to implement is to introduce two new ajax commands:

dialogOpen($content)/dialogSet($content) and dialogClose() so custom ajax commands can control what's happening with the dialog.
Views needs this to implement it's custom logic when the dialog should be closed, and when you should just get another form.

It feels like it would make at least the code easier to read, as just imagine you want to add a new Form to the stack, and you would have to get this information from PHP into JS.

If we go with the ajax commands we certainly should get views converted converted first to new ajax command objects, see #1843224: Convert Views Ajax commands to new Ajax API, but we could work on the generic ajax commands in the meantime (doing now).

Since there's a patch, marking needs review.

Commands should be in dialog.ajax.js, dialog.js should only have the basic code to make dialog work, see #1863478: Split Dialog API and optional integration helpers into separate files. Actually, happy to dup the other one and include the change here.

opened an actual issue to implement a proper API: #1870764: Add an ajax command which makes it easy to use the dialog API in complex cases

Progress is certainly happening on the other issue, so let's postpone this issue first.
It's funny but sadly the dialog API was changed during the issue to not support the slightly more complicated usecase of views :(
This is the reason why we need the other issue first.

Sadly not really, see #1870764: Add an ajax command which makes it easy to use the dialog API in complex cases. We not even need other options, but also proper ajax commands.

This patch does a direct conversion of the current system based on #1870764-102: Add an ajax command which makes it easy to use the dialog API in complex cases. It should be a 100% conversion. It has a few nice improvements:

- Dialogs now reuse the existing commands for open/close. Previous commands for SetViewsForm and DismissForm removed.
- Dialogs are now draggable, since we're using the native jQuery UI header instead of making our own.
- Dialogs *could* be resizable, but since we auto-resize already I'm not sure if that's necessary.
- Lots of cruft code eliminated. Between this patch and the generic modal system, I think we're looking at a net reduction of code just through this abstraction. Of course other places in core will use modals too, so this individual reduction isn't too important.

Every situation I've tested works completely, but there are still a lot of areas for improvement:

- We could move the auto-resizing logic to /core/misc/dialog.js. Seems too useful to be part of Views only.
- We could move the theming of the dialog to system.base.css and system.theme.css. I like the Views-style theming of modals more than the default jQuery UI modals and it'd be nice if they looked the same everywhere.
- We could gut Views' custom form handling and use the approach being researched in #1944472: Add generic content handler for returning dialogs: utilize the router system to eliminate use of nojs/ajax in the paths and set up separate handlers for different versions of the page. (This will be a large undertaking, it'd probably be better to separate this problem).
- There's probably still more cruft code sitting around that no longer does anything that we need to remove. I tried to get all of it, but the Views architecture is so spread out it's hard to be sure that I got it all.

Resize shouldn't be in dialog.js, playing around with it it it seems that the whole views code can be replaced by

and it seems to be working just the same.

We have a debounce function to limit the maddness with resize events.

As for the resizing and centering, the browsers will only use CSS to do that. if you have a #dialogsomething {position:absolute;top:20%;left:20%;bottom:20%;right:20%;} that should work fine in browsers.

Since jquery can't use CSS to do the sizing and all we're stuck with using their API to do it. and that's starting to be tying things down to jQuery UI more than i'm comfortable with.

One could argue that if we make the dialog resizable and draggable It doesn't make sense to keep the centering and resizing on window resize too.

The re-sizing code seems good for re-use if the percentage of height and width, in addition to orientation were parameters rather than hard-coded since that seems to hard-coded to the Views UI specific use case.

Yeah I agree. We could simply re-set the all the size and position settings on resize, just like #21 only with the user-provided values. Of course you'd want to be able to toggle this setting as it might not be appropriate for all situations. I think it would make sense to enable it by default, and have a disable toggle though by overloading the existing $options array (which currently is for jQuery UI options only).

We have a debounce function to limit the maddness with resize events.

Cool, I hadn't seen Drupal.debounce() before. It looks like toolbar library depends on drupal.debounce already, so there's a very high chance it'll be loaded on the page for administrators. Unfortunately since we'll be using dialogs in non-admin situations (i.e. WYSIWYG dialogs), this means an additional HTTP request when the dialog is first opened. Kind of unfortunate, but may be mitigated by other libraries adding debounce.js as a requirement at page-load time.

We could simply re-set the all the size and position settings on resize, just like #21 only with the user-provided values.

In practice, this doesn't work so well. The first problem is that jQuery UI Dialog doesn't support height in percentages, so you can't have a dialog that is "75%" height. Additionally, even if we convert these to pixels on resize, we end up cutting off the buttons at the bottom of the dialog when the window gets too small. The previous implementation always kept the buttons visible. #21's simplification has the same problems. jQuery UI has a dedicated "buttons" wrapper to prevent this kind of problem, but moving our buttons into that wrapper and still having our AJAX behaviors work is going to be tricky business.

(look for // Transform submit in dialog buttons. in the patch) it's ugly but it works.

Yeah I was thinking along the same lines as the only possible solution: hide the button in the original place and have the second button click the first one... Seems like a pretty good trick. I like that your version actually just reuses the existing labels too, that solves any translation problems. The theming for jQuery UI dialogs needs some work, the default CSS for their dialogs have slightly larger/rounder buttons than we use elsewhere in the Seven theme. However since this needs to affect the front-end too... we may want a dedicated stylesheet for dialog theming.

Oh, and regarding the jankiness of button presentation (previously we had a nice blue "primary button" in the dialog, now they're both gray), that problem already has its own issue in #1848292: Consolidate Seven button styles.

There is an issue for the OR dialog #1929070: Refactor views_ui_rearrange_filter_form to remove theme function for table rendering

Is that true even when you apply it in CSS? As in:

Hmm, well I suppose that may be true. If you set a height of "auto", jQuery UI dialogs won't set a specific height property at all, so you could control it with CSS. For some reason jQuery UI Dialogs discard percentage height values but pass through percentage width values

However one problem with CSS-only approach is that the dialog will *always* be that particular height. So a simple dialog that only needs 100px of height ends up displaying 75% of the height, resulting in a huge empty part of the dialog. So even if it's possible using CSS, we'll probably want to use a JS-based solution anyway. The current patch functionality really works pretty well, sorry I haven't rolled it properly so that it can be easily tested. I hope to get to it tonight.

Here's a patch that is pretty much a straight reroll from #30, just combining the two patches (and the file renames) into a single patch that should be testable.

I experimented around with the resizing options quite a bit but I don't think a CSS-only solution is going to work in this situation (I hope I'm wrong). I struggled for quite a while trying to figure out how to do this, but this Views example is really much more complicated that it seems. The trouble is that we have areas inside the dialog that are not definable either in static pixel heights nor percentages. The top and bottom of the dialog show expand as needed to show all the text without scrolling, but the area in the middle needs to scroll the remaining space.

As far as a generic resizing solution goes, I think this is generally solvable through CSS-only after we update to jQuery UI 1.10. Our current version seems to have a bug in it that prevents the use of position: fixed; on the dialog.

I removed jquery.ui.dialog.patch.js, since it will be irrelevant after #1175830: Update to jQuery UI 1.10 so that Drupal dialogs can be tested for accessibility is committed.

Here's an image depicting the problem with the Views dialog. I'm thinking we might be able to solve it through a custom position callback... but I'm really not sure at this point what the best way to handle it is.

Haven't tested yet, but it looks great :)

Nice to have the resizing out of the general dialog and ajax code.

Awesome!

Related: #1832862: Users feel overwhelmed by handler listings

Thanks @dawehner. I can also tack on several other problems:

- The "Selected" item is supposed to populate with the checkboxes you've selected, but it does not.
- The buttons are supposed to update their labels based on affecting the master vs. overridden displays.

I fiddled around with the jerkiness issue a little bit but haven't solved it yet. Part of the reason for the jerkiness is that this version uses the debounce.js file to add a 25ms delay. The previous version was firing on every scroll/resize event, which could be as frequent as 1-5ms. So by effect things will be slightly more jerky but not nearly so CPU consuming.

The last submitted patch, views_convert_modals-1851414-38.patch, failed testing.