Skip overlay initialization if already done [#743908]

The overlay can be made reusable very simply by skipping its own initialization if some other module already did the overlay setup. This can be used to disable / enable the overlay in creative way without intervening with overlay's own code.

A very simple change makes this possible. Just check in overlay_init() whether we have an overlay mode set already. If we do, then we can just skip our own initialization.

Comment	File	Size	Author
#8	overlay-init.patch	3.39 KB	casey
#8
#1	overlay-check-mode-743908-1.patch	3.54 KB	David_Rothstein
#1
	overlay-check-mode.patch	1.16 KB	Gábor Hojtsy

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

David_Rothstein CreditAttribution: David_Rothstein commented 20 March 2010 at 17:27

Status:

Active

» Needs review

File	Size
overlay-check-mode-743908-1.patch	3.54 KB

This makes sense, although some of that is there already, right? (Since overlay_set_mode() already only works the first time you call it...) But I guess this patch would also prevent some of the stuff that's hardcoded into overlay_init() from running as well. It's not totally clear to me what the boundary is between code that belongs inside overlay_init() and code that belongs inside overlay_set_mode() itself.

Anyway, here is an updated patch that fixes a typo in the code comment and also tries to rewrite and expand the code comments for overlay_set_mode() to explain more how other modules can actually enable/disable the overlay. I think it might still need a bit more work, though.

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 5 May 2010 at 12:08

New patch is the same approach that I did above but has added comments on how useful it is :) I think its great. Since it is mostly a docs/typo change, I don't feel empowered to RTBC it, since it feels like RTBC-ing my own patch :)

Comment #3

ksenzee

she/her

Seattle area

CreditAttribution: ksenzee commented 6 May 2010 at 06:00

Status:

Needs review

» Reviewed & tested by the community

Looks great to me. Probably overlay_init() and overlay_set_mode() could largely be combined, but that's an issue for another day.

Comment #4

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 8 June 2010 at 13:10

Status:

Reviewed & tested by the community

» Needs work

+++ modules/overlay/overlay.module	20 Mar 2010 17:22:57 -0000
@@ -56,9 +56,12 @@ function overlay_permission() {
-  if (user_access('access overlay')) {
+  ¶
+  $mode = overlay_get_mode();

trailing whitespace

Powered by Dreditor.