Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#16 | ctools_automodal_paths-1420534-15.patch | 22.66 KB | wojtha |
#1 | 1420534-1-ctools_automodal_paths.patch | 6.02 KB | wojtha |
#3 | 1420534-3-ctools_automodal_paths.patch | 6.68 KB | wojtha |
Comments
Comment #1
wojtha CreditAttribution: wojtha commentedHey 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:
Comment #2
wojtha CreditAttribution: wojtha commentedComment #3
wojtha CreditAttribution: wojtha commentedFollow up
Comment #4
wojtha CreditAttribution: wojtha commentedI've created fork of this module at Github (so I can delete it later in comparison to d.o sandboxes)
Please review the code here:
https://github.com/wojtha/ctools_automodal
Includes:
#1420534: Consider a new hook - hook_ctools_automodal_paths() rather than using hook_menu()
#1439762: regex is too permissive
#1450232: Handle access-denied and page-not-found errors
#1396272: Handle submitted forms
Comment #5
jlyon CreditAttribution: jlyon commented+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.
Comment #6
sinasalek CreditAttribution: sinasalek commentedExcept for php < 5.3 compatibility on line 24 which is easily fixable it works perfectly fine. Thanks wojtha for sharing you work.
Comment #7
Les LimIsn'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.
Comment #8
Dave ReidIt would need to be namespaced with the module name. I'm not opposed to merging this in to 7.x-1.x.
Comment #9
shi99 CreditAttribution: shi99 commentedGreat addition. I would like to see this is merged into the module.
Comment #10
Dubs CreditAttribution: Dubs commentedThanks 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?
Comment #11
jlbellidoI agree with @Dubs
Comment #12
Xen CreditAttribution: Xen commentedWhy 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.
Comment #13
wojtha CreditAttribution: wojtha commentedAfter 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
Comment #14
wojtha CreditAttribution: wojtha commentedComment #16
wojtha CreditAttribution: wojtha commentedComment #17
vistree CreditAttribution: vistree commentedHi 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
Comment #18
Xen CreditAttribution: Xen commentedAgain, 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.
Comment #19
wojtha CreditAttribution: wojtha commented@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:
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.
Comment #20
Xen CreditAttribution: Xen at Reload commented@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.
Comment #21
kopeboy CreditAttribution: kopeboy commentedI think the biggest problem is the tens of thousands people NOT using modals because they don't know how to (there is not a clear winner among modules around this feature, which is much needed) instead of the 1200 already using it.
You could make a new major (incompatible) version that would benefit both if appropriate.
Unfortunately I am not a developer and don't know what I am talking about :(
But I am frustrated for not having a simple solution to open my pages/forms in a f**ing popup/dialog/modal window.
Depending on what the modal window will show you now have:
Then you have this module, which you would think would be the long term solution...
But I don't know how to use it for my needs, usually any simple form provided by third modules (the first thing that comes to my mind is not my custom module, since I am mainly a sitebuilder):
..