I did porting of the popups module for Drupal 7 on purpose of helping the (Mark Boulton) Drupal 7 UX effort, where modal popups (they call them overlays) play a big role. Posting patch here so that your porting of the module to Drupal 7 or hopefully the integration of the module in Drupal 7 will be easier.

I think a module update to Drupal 7 makes it easier to show what this can do and then hopefully integrate the elements one by one, still showing that the module works. However, I do not expect this to be comitted, since you are busy with the 6.x branch, so just offering this as help save you time, since I've already done it anyway.

Attached patch changes:

- .info file to say core = 7.x, files[] list provided
- .js file to be wrapped in jQuery compatibility code
- behaviours changed to use attach method
- clear-block class changed to clearfix
- increasing use of arrays as active menu handler return data required the use of drupal_render on the page content
- drupal_add_js() data structure changed
- drupal_add_css() changes are just cosmetic; used the same variable names as drupal_get_css()
- file_scan_directory() changed to use preg not ereg
- should use the filepath, not filename for constructing CSS and JS paths for later addition to page

This is it. Since the JS is largely independent of the rest of Drupal, it was a quite straightforward port. I did not yet test the meat of the functionality, like detecting dirty pages, but the popups show up properly skinned and can be closed.

Known issue: the popup link does not work as a popup activator anymore after the first click. This is a verified regression from Drupal 6. Don't know yet, how to solve.

CommentFileSizeAuthor
#19 popups drupal7.zip64.07 KBdureaghin
#17 popups-drupal7-port.patch13.87 KBperelman.yuval
#11 popups-7.patch14.59 KBGábor Hojtsy
#6 popups-d7.patch14.12 KBGábor Hojtsy
d7-upgrade.patch12.14 KBGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented

I think we need to be more ambitious with this in Drupal 7. Now that drupal_render has been revamped, I think we could look at doing this correctly. We need a way to translate Drupal pages into other languages other then HTML. If we could receive the Popup callback as a JSON array, then that would be awesome.

Don't get me wrong, a direct port is wonderful, but I think we can do more.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Well, the popup module itself does not need much. It builds its own JSON object with the rendered "main page content" block, a list of JS and CSS files and drupal messages. For popups there is not much of a need for anything else. If you'd push down the whole page array structure (without rendering it on the PHP side as the patch does), then you'd need the full drupal_render() stuff implemented in JS, which would certainly be fun, but probably highly pointless, no?

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented

What I mean to say is that we should push some of this into Drupal core ;-) .

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Definitely, we should push into core. The reason I am doing it this way is to get stuff for a working prototype without waiting on the bazillion components being committed to core.

yched’s picture

yched CreditAttribution: yched commented

Probably not the right place to post this but : one issue with D6 popups are HTML ids clashes between either the underlying page and the popup, or two stacked popups - breaking id-based JS behaviors.
Bringing popups in core and extending their use might need some rethinking around the way ids are assigned and behaviors are attached.
Please forgive me if this is already discussed somewhere else.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Active » Needs review
FileSize
popups-d7.patch14.12 KB

Looked into some problems with our port with Paul Lovvik. He found that the JavaScript matching was not right in where it merges JS files from multiple requests. Updated patch attached. Tested with many of the popups tests, and it worked fine. Not clicked through all per say.

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented

Here's the core issue: #374646: Popbox (Popups Lite): Adding Modal Dialogs to Confirmations. I think its original goal in there was to just bring in confirmation popups.

The patch you have here, however, looks like pure DRUPAL-7--2 ready awesomeness.

paul.lovvik’s picture

paul.lovvik CreditAttribution: paul.lovvik commented

Note that this port also requires the IE fix (http://drupal.org/node/419974).

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

@Rob Loach: yes, I am tracking and worked on some of the existing D7 popups patches already. I believe the work was fragmented to a point where it was quite hard to get any of that into core, and unfortunately @starbow seems to have been burned out in that and stopped working on core integration. So to make it easier for Mark Boulton Design to test stuff, we decided to first port the popups module and then work on getting its pieces into core, parallel to user testing which will hopefully validate its importance :)

paul.lovvik’s picture

paul.lovvik CreditAttribution: paul.lovvik commented

I noticed that Drupal behaviors attached to the page being displayed in a popup can cause trouble once the popup is dismissed. I submitted a core patch for one such issue here: #478874: JavaScript error occurs after displaying the /admin/build/block in an overlay using AJAX.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
popups-7.patch14.59 KB

Updated popups stack removal code from paul.lovik. Works even better with Drupal 7.

starbow’s picture

starbow CreditAttribution: starbow commented

@Rob Loach & @Gabor: I do want to get back into the Drupal 7 game, but I am not sure when I am going have the time. Are you two interested in getting cvs access to work on the D7 version?

BTW - you lose a lot of power if you rip out popup stacking. I would think the way to go is id munging of the elements in the non-active popups.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

@starbow: my main interest in this port was a possible core inclusion. I've been posting about contemplating core inclusion of popups or modal dialog API for Drupal 7 here: http://hojtsy.hu/blog/2009-jun-17/two-alternatives-d7ux-overlay-implemen... It would be great if you could chime in.

astutonet’s picture

astutonet
Location Sao Paulo, SP
CreditAttribution: astutonet commented

Any changes in this version?

We have an oficial 7.x-dev version?

Tks

torsti’s picture

torsti CreditAttribution: torsti commented

Is it possible to have 7.x version released?

BeaPower’s picture

BeaPower CreditAttribution: BeaPower commented

Drupal 7 version ready yet?

perelman.yuval’s picture

perelman.yuval CreditAttribution: perelman.yuval commented
FileSize
popups-drupal7-port.patch13.87 KB

Hi all.
i also make a port to drupal 7, the port is for the 6.x-2.0 branch, and it is working great.

perelman.yuval’s picture

perelman.yuval CreditAttribution: perelman.yuval commented

@starbow first thanks for the great module, for all the popups modules in the area i find your's the most easy and stable one.
It will be great if you can review me patch and give my git access so i can make a D7 version.
thanks again

dureaghin’s picture

dureaghin CreditAttribution: dureaghin commented
FileSize
popups drupal7.zip64.07 KB

Drupal 7

astutonet’s picture

astutonet
Location Sao Paulo, SP
CreditAttribution: astutonet commented

Finally, I think that we have a dev version.

I tested some resources with #19 version, and they works.

But I found problems with the skins. They simple don't works.

Tks.

tsi’s picture

tsi CreditAttribution: tsi commented
Status: Needs review » Reviewed & tested by the community

+1 this is working great ! absolutely deserve a formal branch.

purabdk’s picture

purabdk CreditAttribution: purabdk commented

Does not work with drupal 7.15