This way we could add support for things like options for the modals. We can also provide backwards-compatible support for any menu callbacks that have 'modal' => TRUE.

Files: 
CommentFileSizeAuthor
#16 ctools_automodal_paths-1420534-15.patch22.66 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#1 1420534-1-ctools_automodal_paths.patch6.02 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#3 1420534-3-ctools_automodal_paths.patch6.68 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

wojtha’s picture

Category:feature» task
Status:Needs review» Active
StatusFileSize
new6.02 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Hey Dave, thx for this module.

Here is a patch which uses this idea. I've created two hooks hook_modal_paths() and hook_modal_styles().

In hook_modal_styles you can define your custom modal styles (format of these settings is described e.g. here: http://drupal.org/node/872072#comment-3391816) and use them in the hook_modal_paths options.

I've extended the ctools_automodal_get_form() handler, so now it handles forms nicely. In options you can define what is going to happen after the form submission. You can display confirmation page, autoclose the form, redirect to the path from form_state or you can reload the original page.

Note that the hook_modal_paths wasn't fully integrated so ['modal'] = TRUE in the hook_menu or hook_menu_alter() is still needed.

Example implementation:

<?php
/**
 * Implements hook_modal_paths().
 */
function eco_user_modal_paths() {
 
$paths = array();

 
$paths['user/register'] = array(
   
'style' => 'eco-user-signup',
   
'redirect' => 'user',
   
'close' => TRUE,
  );

 
$paths['contact-us'] = array(
   
'style' => 'eco-user-contact',
   
'confirm' => array(
     
'title' => t('Thank you'),
     
'text' => t('Your message has been sent.'),
    ),
  );

  return
$paths;
}

/**
 * Implements hook_modal_styles().
 */
function eco_user_modal_styles() {
 
$styles = array();

 
$styles['eco-user-contact'] = array(
   
'modalSize' => array(
     
'type' => 'fixed',
     
'width' => 500,
     
'height' => 500,
    ),
  );

 
$styles['eco-user-signup'] = array(
   
'modalSize' => array(
     
'type' => 'fixed',
     
'width' => 600,
     
'height' => 700,
    ),
  );

  return
$styles;
}

/**
 * Implement hook_menu()
 */
function eco_user_menu() {
 
$items = array();

 
$items['contact-us'] = array(
   
'title' => 'Contact Us',
   
'description' => 'Contact Us',
   
'page callback' => 'drupal_get_form',
   
'page arguments' => array('eco_user_contact_us_form'),
   
'access arguments' => array('access content'),
   
'type' => MENU_CALLBACK,
   
'modal' => TRUE,
  );

  return
$items;
}

 
/**
 * Implementation of hook_menu_alter().
 */
function eco_user_menu_alter(&$callbacks) { 
 
$callbacks['user/register']['modal'] = TRUE;
}
?>
wojtha’s picture

Status:Active» Needs review
wojtha’s picture

Category:task» feature
StatusFileSize
new6.68 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Follow up

wojtha’s picture

Category:task» feature
Status:Active» Needs review
jlyon’s picture

+1 to #4. I have been using the github forked version of this, and it works great (except for the php < 5.3 error outlined on http://drupal.org/node/1439762#comment-6270388). It would be great to start a 2.x branch of this module with these changes so that we can start tracking issues related to this collection of patches in the issue queue.

sinasalek’s picture

Except for php < 5.3 compatibility on line 24 which is easily fixable it works perfectly fine. Thanks wojtha for sharing you work.

Les Lim’s picture

Status:Needs review» Needs work

Isn't there a naming convention about namespacing hook implementations with the module name? HOOK_modal_paths() isn't likely to be accidentally implemented, but I'd rather be safe than sorry.

Dave Reid’s picture

It would need to be namespaced with the module name. I'm not opposed to merging this in to 7.x-1.x.

shi99’s picture

Great addition. I would like to see this is merged into the module.

Dubs’s picture

Thanks so much for this great module and for the additional functionality provided in the patches.

The GitHub version works really well for me with no problems.

Could you please release the forked version on d.o so others can benefit from these additions?

jlbellido’s picture

Issue summary:View changes

I agree with @Dubs

Xen’s picture

Why exactly is a new hook needed? The modal key could just as well be an array containing additional options.

I consider the fact that the modal definition is *on* the menu entry quite a feature.

wojtha’s picture

StatusFileSize
new23.63 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in ctools_automodal_paths-1420534-13.patch.
[ View ]

After 3 years I need that functionality again so I've fixed some issues from github and and I've made a new branch 'prefixed_hooks' to conform @David's request to give proper names to the hooks.

hook_modal_paths() => hook_ctools_automodal_paths()
hook_modal_paths_alter() => hook_ctools_automodal_paths_alter()
hook_modal_styles() => hook_ctools_automodal_styles()
hook_modal_styles_alter() => hook_ctools_automodal_styles_alter()
hook_modal_error_alter() => hook_ctools_automodal_error_alter()

Includes patches from @greggles and @jlbellido

wojtha’s picture

Title:Consider a new hook - hook_modal_paths() rather than using hook_menu()» Consider a new hook - hook_ctools_automodal_paths() rather than using hook_menu()
Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 13: ctools_automodal_paths-1420534-13.patch, failed testing.

wojtha’s picture

Status:Needs work» Needs review
StatusFileSize
new22.66 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
vistree’s picture

Hi wojtha,
can I use your branch to make user_profile_form (user edit) work in modal form?
I opende a parallel thread here:
https://www.drupal.org/node/2417509

Xen’s picture

Again, why does this have to introduce another hook? The modal key could just as well be an array of options, with TRUE just using defaults for everything.

wojtha’s picture

@Xen the reason is you can add easily modal behaviour to existing menu items. Plus I think I had some implementation problem by using hook_menu and using separate hook was easier to me and solved the issue I had. But it's more than 3 years so I'm not able give you any detail right now...

But read the original issue, even the author of the module wanted to move it to separate hook:

This way we could add support for things like options for the modals. We can also provide backwards-compatible support for any menu callbacks that have 'modal' => TRUE

So there must be some problem why hook_menu cannot be used for adding more options and features. But I again, after 3 years I'm not able to say why... Maybe Dave Reid will remember.

Xen’s picture

@wojtha
"the reason is you can add easily modal behaviour to existing menu items."

That's what hook_menu_alter() is for.

"But read the original issue, even the author of the module wanted to move it to separate hook:"

The first word of the issue title is "consider". There has been no condsidering in this issue. I've seen no arguments for why using hook_menu() is a problem. The closest we get is "This way we could add support for things like options for the modals.", but as already mentioned, a new hook is not required for that.

There's close to 1200 uses of this module, and due to the nature of the module, each one of them have had a developer add the "'modal' => TRUE," somewhere. Creating a new hook means 1200 installations is suddenly legacy that should be updated. Sure, we can add compatibility code to support the old way, but then you're just introducing cruft into ctools_modal.

Maybe there IS a problem in using hook_menu(), but then we need to identify that problem. There could be an alternative solution that doesn't require a total rewrite of the documentation.