Adding on BeforeOpen and on CloseDone events to Overlay [#1918692]

I have made an experimental module that makes the overlay able to remember and restore the scroll height of the parent window once the overlay closes. This also works, when the parent window is being redirected e.g. after a save action, because it stores the scroll value in the window.name variable.

The module (Overlay helper) can be found here : https://drupal.org/sandbox/prinds/1918678

But in order for it two work, two extra javascript events needs to be triggered in the overlay-parent.js:
- drupalOverlayBeforeOpen
- drupalOverlayCloseDone

These are placed early and late in the process of opening and closing, ensuring that the body height of the parent window is intact.

I will attach a patch that adds the events..

Will it be possible to accept this patch into the overlay module?

Comments and suggestions on the overlay helper module are also welcome.

Comment	File	Size	Author
#10	extra_events_for_overlay-1918692-10.patch	667 bytes	prinds
#10
#1	extra_events_for_overlay-1918692-1.patch	669 bytes	prinds
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

prinds CreditAttribution: prinds commented 15 February 2013 at 23:40

File	Size
extra_events_for_overlay-1918692-1.patch	669 bytes

Comment #2

prinds CreditAttribution: prinds commented 25 February 2013 at 14:03

Status:

Active

» Needs review

Comment #3

prinds CreditAttribution: prinds commented 11 March 2013 at 22:21

Title:

Add two extra javascript events - drupalOverlayBeforeOpen and drupalOverlayCloseDone

» Restoring scroll position on overlay close - a solution

Bump - No one else want to be able to restore scroll position af overlay is closed?

Perhaps this should/could be a part of the overlay module?

Please take a look at the experimental module here https://drupal.org/sandbox/prinds/1918678 and post your comments and suggestions..

Comment #4

prinds CreditAttribution: prinds commented 22 March 2013 at 11:29

Bump..

I would really like to get some feedback on this.. Is this useful at all?

Comment #5

yannickoo

Berlin

CreditAttribution: yannickoo commented 24 March 2013 at 10:45

Status:

Needs review

» Needs work

+++ b/modules/overlay/overlay-parent.jsundefined
@@ -204,6 +205,8 @@ Drupal.overlay.close = function () {
+  ¶

Whitespaces ;)

Comment #6

prinds CreditAttribution: prinds commented 26 March 2013 at 20:11

I'm not sure I understand what you mean?

What needs to be done?

Is it a newline before "return true" in Drupal.overlay.close?

Comment #7

prinds CreditAttribution: prinds commented 3 May 2013 at 14:12

Status:

Needs work

» Needs review

Setting this back to needs review...

Please be more specific, if more work needs to be done.

Comment #8

yannickoo

Berlin

CreditAttribution: yannickoo commented 13 August 2013 at 13:46

Status:

Needs review

» Needs work

What I mentioned in #5 were the two spaces. Install Dreditor then you also can see them when reviewing patches :)

Your patch only adds two triggers but no functions or binds, your JavaScript does.

Comment #9

yannickoo

Berlin

CreditAttribution: yannickoo commented 13 August 2013 at 13:48

Status:

Needs work

» Closed (duplicate)

You also should come to #1468902: Restore scroll position after closing overlay because it was posted before your post :)

Comment #10

prinds CreditAttribution: prinds commented 13 August 2013 at 19:29

File	Size
extra_events_for_overlay-1918692-10.patch	667 bytes

I see.. The whitespaces are now gone in the attached patch :)

The triggers are needed for the functionality of course. The reason I put the rest in a seperate module is because I thought most people wouldn't be interested in restoring scroll height (otherwise this would probably have been developed much sooner).

But it would of course be no problem to add the whole code to the overlay module itself.

Anyone interested in a patch that does that?

Thanks for your help...

Comment #11

NaX CreditAttribution: NaX commented 23 August 2013 at 10:10

Title:	Restoring scroll position on overlay close - a solution	» Adding on BeforeOpen and on CloseDone events to Overlay
Status:	Closed (duplicate)	» Needs review

I don't think this issue is really about the scroll position. The scroll position stuff is what instigated the changes to overlay. The patches are for adding Before and After patches to overlay so I have updated the Title and I am re-oping this issue.