The Overlay module is hardwired to only work on admin pages (via hook_admin_pages()), even though this coupling can be loosened with a very small code change. This would allow modules to open non-admin pages in an overlay without having to install on the contrib Lightbox-type modules.

This patch introduced two new hooks, hook_overlay_paths() and hook_overlay_paths_alter(), in addition to the existing hook_admin_paths() and hook_admin_paths().

The new hooks were actually proposed in #610234: Overlay implementation, but the hook_admin_paths()/hook_admin_paths_alter() names were considered more appropriate, because other modules could be interested in what are admin paths. This is true — but this is based on an assumption that the overlay works for all admin pages and for admin page only.

What do you think?

CommentFileSizeAuthor
overlay-admin-1.patch18.96 KBc960657

Comments

robloach’s picture

robloach
he/him
commented

This patch introduced two new hooks, hook_overlay_paths() and hook_overlay_paths_alter(), in addition to the existing hook_admin_paths() and hook_admin_paths().

What if we were to "fix" hook_admin_paths() by renaming it to hook_overlay_paths() and adding the features you mentioned?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Well, the reason we didn't call it hook_overlay_paths() to begin with is we wanted to genericize this capability for potential use outside of Overlay. Overlay may not be the only module that wants to do some special reaction on admin pages. We'd also have to start adding the word "overlay" in several key modules including node, system, user, etc. etc. which I anticipate would go over like approximately 1,000 metric tonnes of radiation-soaked lead.

Christian, is there a reason you can't just use hook_admin_paths_alter() for your case? Adding two hooks that do almost the same thing strikes me as really hard to document properly.

Another way to approach this might be a configuration settings form for Overlay, to allow people to put in their own paths for what it should show up under. Maybe?

c960657’s picture

c960657 commented

Just for the record: I am not suggesting that core modules should implement the proposed hook_overlay_paths(). As long as they implement hook_admin_paths(), the Overlay module takes care of adding the overlay to the admin pages.

Also: The patch may seem big, but a lot of the changes are changes in wording of variable names and UI texts in order to reflect that the Overlay is not only for administrative pages.

Christian, is there a reason you can't just use hook_admin_paths_alter() for your case? Adding two hooks that do almost the same thing strikes me as really hard to document properly.

I am working on a project that uses modal overlay (aka lightboxes) quite a lot for forms visible to end-user (e.g. log in, create/edit account, etc.). These forms are not admin forms and are not intended to use the admin theme.

I guess I could expose them as admin paths (even though they are not) using hook_admin_paths_alter() and then deal with any unwanted side effects from modules that use path_is_admin() or path_get_admin_paths(). But this seems like a hack.

Modal overlays have become quite common on the web, also for non-admin stuff. Implementing a modal overlay is not simple, and I haven't found a contrib module that works out of the box without a lot of manual URL-rewriting etc. I had hoped I could use the Overlay module for this, but perhaps it is simple just not the right tool for the job. But why couldn't it be in D8?

Another way to approach this might be a configuration settings form for Overlay, to allow people to put in their own paths for what it should show up under. Maybe?

That could work. I chose the hook-based approach because it seemed more in line with the approach for admin pages. But i think a variable-based solution would solve my problem also. This would be a smaller code change.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Oh, ok. Now that I actually read the patch, I understand what's going on.

So this effectively de-couples the two functionalities so hook_admin_paths() refers to "where should the admin theme show up?" and hook_overlay_paths() refers to "where should the overlay show up?" That seems like a pretty clear distinction. And it does it without any required changes in any of the other modules exposing admin paths. Nice!

I like this, because it makes the Overlay more generally useful, and potentially a replacement for Lightbox out of the box. Rad! It's doubly great since, as much crabbing as the Overlay gets, it's actually a pretty sound piece of front-end engineering (it better be since we re-wrote it at least twice :D) that handles things like back-button presses, etc.

Count me as +1. I'll try and give this a more in-depth review later, and maybe, maaaayyybeee sucker ksenzee into one too. ;) No promises.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Overlay theme is hardwired to admin pages » Allow Overlay to be used outside admin screens

Re-titling slightly to be more clear.

David_Rothstein’s picture

David_Rothstein commented

This looks closely related to #1220234: Introduce path_get_tagged_paths, and allow the overlay to be used on non-admin pages, which takes a slightly more generic approach to the same problem (one which does not require the overlay module to maintain its own path-matching code). Should this issue be merged into that one?

Note that for D7 there's a contrib module at http://drupal.org/project/overlay_paths that looks like it provides this functionality, although I haven't tried it myself.

c960657’s picture

c960657 commented
Status: Needs review » Closed (duplicate)

Should this issue be merged into that one?

Yes, it's a slightly different approach, but I think this ticket is a duplicate. Let's continue discussion over there #1220234: Introduce path_get_tagged_paths, and allow the overlay to be used on non-admin pages.

Note that for D7 there's a contrib module at http://drupal.org/project/overlay_paths [...]

Oh, very nice. I have added it to my project.