When comparing modalframe vs popups the main item in favor for popups is its ease of implementation.

Popups provides a simple hook that allows you to define which links will have popups, and provides a very graceful degradation when popups is not enabled.

Is there a reason no such functionality exists currently in modalframe?

The way that modalframe is structure requires that you customize each popup that you want to run on your site, if in forms, you need to add function calls to the submit handlers.

I'm working on a patch that adds a hook to modalframe. This will allow module developers to implement modalframe easily.

I will post a patch as soon as I have it presentable.

CommentFileSizeAuthor
#22 modalframe.patch4.67 KBcorey.aufang
#12 modalframe.patch2.02 KBcorey.aufang

Comments

markus_petrux’s picture

markus_petrux commented

I did not implement a developer interface such as the one in Popups API because my initial goal in this project was mostly focused on providing the kind of functionality it currently offers. It was a particular need, and I never thought it was going to happen what has really happened since this module was published. It has been compared to Popups, and it is now being integrated into D7 as part of the D7UX initiative.

Since it was made to server a particular need, I stopped coding once the initial goal was accomplished. Now, I realize that it could go beyond, if that can help others use it more easily... so I'm happy to see this request. At a first sight, I would try to implement this extension in a way that is easy to swap from Popups API to Modal Frame API. But since this is now being implemented in D7, then I think it would be nice to see if what we do here is compatible to what it is being made for D7 (#517688: Initial D7UX admin overlay), so that the upgrade path from D6 + Modal Frame API to D7 is as easy as possible.

corey.aufang’s picture

corey.aufang commented

I agree that providing a simple upgrade path from a new api to D7 would be ideal.

One improvement would be to make drupal aware that it is being loaded in an overlay. Looking in #517688: Initial D7UX admin overlay they append render=overlay as a GET parameter when loading the target page.

If the paramter is set, we run modalframe_child_js() else we run modalframe_parent_js() .

Automatically detecting that a form is fully processed I tested this modification:

function modalframe_form_submit($form, &$form_state) {
  if ($form_state['redirect']) { //redirect should only be set if the form has been completed so we can go ahead and close the dialog
    modalframe_close_dialog();
  }
  if (!empty($GLOBALS['modalframe_close_dialog'])) {
    $form_state['redirect'] = FALSE;
  }
}

I have not created a hook yet for deciding which links should have modals, wanted to get your input on the other changes before I did anything further.

One more thing, what are you using for dragging a frame around? It seems to lose tracking if you move it too quickly (not tested in anything other than FF3.5 yet).

markus_petrux’s picture

markus_petrux commented

I like the idea of calling modalframe_child_js() automatically when render=overlay is in the URL.

But we cannot call modalframe_parent_js() because this will add jquery ui stuff to all pages. We should only do this when triggered by *something* that tell us we want to be able to use modal frames on this particular page. Also, we may still want to be able to open modal frames from child modal frames.

We could add render=overlay to the URL when opening the dialog, but we cannot do it on the server side to all URLs unless we find a way to control which URLs we really want to keep on the modal frame. This is now implemented in the client-side of modal frame api, child.js, based on CSS classes.

Another requirement here would be that we cannot break the modules that are currently using Modal Frame API.

markus_petrux’s picture

markus_petrux commented

Re: "One more thing, what are you using for dragging a frame around? It seems to lose tracking if you move it too quickly (not tested in anything other than FF3.5 yet)."

This is a jQuery UI issue. I also noticed this behavior in FF, but IE and Opera do not lose the tracking. The rendering in FF and IE is quite fast, but Opera is slower.

I think one way to enhance this would be to override the draggable element used by jQuery UI dialog, but it was not possible when I last checked. If we could alter the draggable object, we could use a helper frame to show the border of the frame while dragging, and this behavior could be optional (governed by arguments to the open method of the parent code). Maybe this will be possible in future versions of jQuery UI.

CPyle’s picture

CPyle commented

But we cannot call modalframe_parent_js() because this will add jquery ui stuff to all pages. We should only do this when triggered by *something* that tell us we want to be able to use modal frames on this particular page.

In the case of the popups api, this is handled through the array you return in your module hook. The array is keyed by the url(s) you want your popup links to appear on, using wildcards. You could make a key like ['user/*/edit'], which would cause the "applying popup logic to links" javascript to be run only on user edit pages.

I'm assuming @corey.aufang would probably want to adopt a similar approach.

corey.aufang’s picture

corey.aufang commented

This is something I overlooked, and I agree that it really does make sense to only call modalframe_parent_js() on pages that actually will have popup links.

Using a hook, we could get data back in this form:


array(
  'node/*' => array(
    'a[href^="node/"][href$="/edit"]' => array(
      'draggable' => FALSE,
      'width' => '800px',
    ),
  ),
);

This array means that for pages that match node/* to first run the function modalframe_parent_js() and then run drupal_add_js setting to tell the JS to look for links that match the jQuery selector 'a[href^="node/"][href$="/edit"]' and to use those as modal frames

corey.aufang’s picture

corey.aufang commented

I also agree that adding render=overlay to the URL when opening the dialog is the only feasible solution.

What did you mean by

This is now implemented in the client-side of modal frame api, child.js, based on CSS classes.

?

markus_petrux’s picture

markus_petrux commented

By CSS classes I mean "modalframe-exclude". See child.js, method Drupal.modalFrameChild.behaviors.parseLinks(). That is, you can add the class "modalframe-exclude" to any link that you want to open on the same modal frame a new window. The rest of the links will open on new window the same modal frame.

I coded this is a way that it is the code using modal frame api who decides where it wants to use the CSS class "modalframe-exclude".

This behavior has been changed in the patch for D7. That one could be introduced here, but still keeping the current behavior to not break existing modules that use it.

[EDIT] fixed sentence where I described the opposite behavior. Apologies for the confusion.

corey.aufang’s picture

corey.aufang commented

Excluding links from spawning modal frames when they are already in modal frames seems like it should be an option that you can set in when you attach the action to a link.

So you could have something like:

array(
  'node/*' => array(
    'a[href^="node/"][href$="/edit"]' => array(
      'draggable' => FALSE,
      'width' => '800px',
      'exclude' => TRUE,
    ),
  ),
);

Expecting someone to go through and for each link add a class to exclude seems a bit redundant.

markus_petrux’s picture

markus_petrux commented

Sorry, I made a mistake in my previous comment. Edited.

IIRC, that class in the D7 patch is "overlay-escape". But they close the overlay and would prefer not to. It would be a problem when the modal frame renders a view that contains links to other nodes. If I click a node title, I want to open it on new window by default, but still keep the modal frame open so that I can keep browsing, etc. This is used in Node Relationships module, Search and reference feature, for example.

Sadly, I have my hands dirty right now, so I cannot spend too much time coding here.

corey.aufang’s picture

corey.aufang commented

By tomorrow I'm going to try and get a patch together with some of the mentioned functionality.

corey.aufang’s picture

corey.aufang commented
Status: Active » Needs review
StatusFileSize
new modalframe.patch2.02 KB

Here is the patch with the functionality that I was mentioning.

This is rolled against dev.

I did not include changes to match comment #10.

CPyle’s picture

CPyle commented 
+function hook_modalframe() {
+  return array(
+    'node/*' => array(
+      'a[href$="/edit"]' => array(
+        'draggable' => FALSE,
+        'width' => 1200,
+      ),
+    ),
+  );
+}
+

Did you forget to take this out before you made the patch? Probably should not have any hooks on by default...

corey.aufang’s picture

corey.aufang commented

If you notice, the function is hook_modalframe, as to provide an example.

markus_petrux’s picture

markus_petrux commented
Status: Needs review » Needs work

1. hook_modalframe() would have to be placed into a separate file modalframe.api.php or something similar.

2. We cannot do the following because there may be situations where redirect is set, but we do not want to close the dialog. Think about a wizard, a node preview, a view where we are updating exposed filters, etc.

+  if ($form_state['redirect']) {
+    modalframe_close_dialog();
+  }

Also, we would have to check for isset($form_state['redirect']).

3. Before doing the following, we would have alter parent.js to add this to the URL of the child window, but we also need to define when and how we can add render=overlay to URLs automatically from server-side. What can we use to trigger this in a way that is fully compatible with current behavior?

+  if ($_GET['render'] == 'overlay') {
+    modalframe_child_js();
+  }

Also, we would have to check for isset($_GET['render']).

4. Could we move hook_init() and this functionality to a separate module? That way, this feature would be really optional. If I don't need it, I have the chance to not enable this module and I can save a few execution cycles because of module_invoke_all(). Such a module could be placed inside the modules subdirectory of this package, and could be named modalframe_extensions.module, modalframe_extras.module, modalframe_autoloader.module, modalframe_funforall.module, or something else... I don't like these module names, but it would be nice if this feature was optional, and using a separate module would be easier to implement, I think.

looplog’s picture

looplog commented

I was going to open a separate feature request, but it appears this thread may cover my need.

Popups api allows giving any link the class of "popups" and have it open in a popup. Where this really shines is in combination with the CCK link module, which allows any CCK link to be assigned this class, thus opening up the door for popup/link integration with views and so on.

That being said, I find modalframe to be the better of the two modules. Does the feature above provide such a class? If not, is creating a custom module my only option at the moment for creating an edit link exposed to views for any node? (sorry if this is thread hijacking, but it appears that this thread seems to be connected).

edit: I had a look at the patch above and I think I understand that it allows appending &render=overlay to a link to open in a modalframe. Is this correct?

markus_petrux’s picture

markus_petrux commented

@omjn: Yes, I think this feature will make that possible. But it still needs work. I would say we are still in design stage here. Your input is much appreciated. I didn't know cck link had integration with popups, and it looks like a nice think to cover here as well.

looplog’s picture

looplog commented

There's no hard-coded integration between CCK link and popups, but because popups api provides a general "popups" class and CCK link allows arbitrary classes to be asigned to links upon field creation the integration flows from there. So its more of a synergy... I imagine such a class is all that would be needed for modalframe to provide replacement functionality.

corey.aufang’s picture

corey.aufang commented

In response to comment #15

3. The patch should have included changes to parent.js that automatically append render=overlay to the url that is used for the modalframe.

I'll create a module to contain this functionality and try to get it uploaded by monday sometime.

markus_petrux’s picture

markus_petrux commented

Re: "I'll create a module to contain this functionality and try to get it uploaded by monday sometime."

Maybe the part that could be isolated into a separate module because of the need to output the js to all pages, could be resolved adding just a small javascript behavior here that lazy loads the rest of the required javascript libraries on demand. So maybe we do not need a separate module. If we do, this module would have to be part of the same package. It could be located in the modules subdirectory of the modalframe package. If it was a separate package, we would be added a potential layer of problems because compatibility issues between both projects could easily arise if the API is changed/extended.

corey.aufang’s picture

corey.aufang commented

Sorry, let me clarify.

I will upload a patch that provides the functionality in a module in the modules sub-directory.

corey.aufang’s picture

corey.aufang commented
StatusFileSize
new modalframe.patch4.67 KB

Here is a new patch, it moves the functionality to a new module in the modules sub-directory.

Since this has been moved to another module, I added a new feature I thought would be useful. It should be explained in the README.txt file provided.

I know that the documentation should be expanded before being committed, but I would like some feedback first before expanding the documentation.

corey.aufang’s picture

corey.aufang commented
Status: Needs work » Needs review
markus_petrux’s picture

markus_petrux commented

Just wanted to mention that I'm busy trying to finish the project I'm working on. I'll try to come back to review this as soon as possible. It may take me a month or so.

corey.aufang’s picture

corey.aufang commented

I've made some additional improvements to the functionality.

Also I fixed a bug where frames were not being properly passed the content url.

Wanted to make sure there was still interest in this before making a patch.

markus_petrux’s picture

markus_petrux commented
Status: Needs review » Closed (won't fix)

Hi all,

I've been thinking about this kind of enhancements, and trying to play with different places where the Modal Frame API could enhance the usability of day to day tasks in a Drupal installation. I've found that every place where this could be done has its own particularities, and that makes the task to provide something like it is being discussed here quite complex. It is not so easy to abstract an API that can cover all possible situations, because you'll find many times yourself you may need to implement additional stuff for each place where you want to use the Modal Frame. At least, if you want to cover each feature as well as possible.

On one side, having an API to do such task imposes certain limitations. On the other, we can create mini-modules that do this job by just using the Modal Frame API as it is now. Oh, please note that I've been adding more features, as well as fixing all known bugs.

So... I ended up creating a separate project to host mini-modules that use the Modal Frame API to implement all these things. Well, I just implemented a few mini-modules. But that's just a start, and everyone is welcome to contribute. Please, check it out:

http://drupal.org/project/modalframe_contrib

It requires latest development snapshot of Modal Frame API (I just committed a few things today).

For the moment, I'm going to mark this issue as won't fix. The method to universalize the Modal Frame API ala Popups API can still be implemented on a separate project. @corey.aufang: you're welcome to do so, or maybe you can also find the new contribs package a good enough approach. Please, look at how each mini-module has been implemented. I really think all this stuff is quite hard to deploy using an API similar to the one provided by Popups API. There's always a little thing that makes it different.

Also, using separate mini-modules, the site administrator can only enable those that are needed. So, this is also a method to not increase too much the memory requirements to the site, I think. You can enable only what you need.