Review/consider layout architecture

[MrDys]

CODEBASE-309: In current BL-Rails3, all Blacklight actions use a "blacklight" layout, which is included in the plugin. The point of this is to provide an out-of-the-box fully functional and aesthetically pleasing layout for just BL controllers, without touching other controllers in the app.

However, some (cbeer) have found this confusing to figure out what's going on. Also, a very typical use case is you are going to want blacklight controllers to use your overall application layout same as any other controllers, and it's somewhat trickier to do this than you might want for such a common case.

Keep in mind the BL layout includes some hooks for actual functionality (like link rel=alternative auto-discovery), as well as aesthetic design. You can certainly copy those hooks into your own layout. While some (dfunk) choose to over-ride and not use that functionality, I think it's important people realize they are doing so and choosing to do so, the default path should ideally not lead people doing this by accident and losing functionality.

In Rails2, BL provided it's own "application" layout in the plugin, which the demo app didn't automatically over-ride locally, but you could over-ride it locally. This won't work in Rails3 version, when the initial app is going to already have an application layout.

Possible options I can think of:

1. Leave it as it is but document it better.
2. Leave it substantially as it is, but refactor the methods involved so you can over-ride a single method with the name of the layout, not needing to over-ride additional logic about XHR unless you want to.
3. Make all BL controllers use ordinary default layout (returning 'true' from method, meaning they'll ordinarily use application.html.erb). But GENERATE an application layout into the local app. The user can then customize this layout, or replace it entirely, but it'll be right there in front of them making it less likely they'll remove the functional hooks without meaning to. Downside is if they already have their own layout they'll have to manually review the diff if they don't want to overwrite it entirely.
4. Variation on #2. Note that Rails 'default' layout lookup will first look for a layout with the same name as the controller; then do the same with any controller superclasses up the chain, and only if none are found resort to layout named 'application'. If we make all the BL controllers have a common superclass for instance BlacklightController, then those controllers default lookup will FIRST look for a layouts/blacklight.html.erb and only use the application layout if not found. We could provide the layout at layouts/blacklight.html.erb automatically in the BL gem, OR instead we could generate it into the local app -- the user could choose whether to generate it into layouts/blacklight (just applying to BL controllers) or layouts/application (taking over whole application). Or some combination (it could be in the plugin as layouts/blacklight, but you could choose to generate it as layouts/application).

[MrDys]

Original reporter: jrochkind

[MrDys]

jrochkind: good enough for now.

[MrDys]

jrochkind: Okay, I've been unable to come up with a solution I'm totally happy with. Largely keep running up against a road block in a missing functionality in RAils --- once you've defined a method to determine the layout, it's not possible for that layout to return (eg) a 'true' value meaning "use default layout lookup logic", you NEED to return a specific layout name. That kept getting in the way of all the various ideas I had for elegantly have BL provide a default layout for BL controllers, but make it easy to turn off etc.

So I'm going to do the simplest thing that will improve the most egregious problem with the current setup. There's currently a choose_layout method which you can over-ride in your own application_helper, but you have to over-ride more than you want (logic for suppressing layout for XHR etc). I'm going to factor out a #layout_name method that you can over-ride, over-riding just the layout name without having to copy/fork the logic for supressing in case of XHR too.

Will commit momentarily.

[MrDys]

jrochkind: Some more thoughts on layouts.

*. It is in fact possible to do quite substantial_look and feel changes using the built-in layout, just using CSS as well as localizing some of the partials the layout calls out to (like user_util_links or whatever the navbar is called). This is in fact what I've done (or what I should have done -- I think I localized it to add google analytics, before realizing GA could be added using unobtrusive js instead).

• However, certainly some people will want to localize the layout itself, and we indeed want to make that easy.
• Whether the user localizes the layout or not, I suggest the following will be a very common typical use path, which we want to make as easy as possible:

-> Using the same layout BOTH for blacklight controllers AND for all the rest of your controllers, AND having this layout include the standard Blacklight functionality.

That's not the only case we want to support, but I think it will be a very common and typical one, especially for less-expert users. I think it's one we want to make very simple. Currently, whether you localize the layout or not, trying to do the above pretty typical case is a bit more confusing/complicated than it should be, and also involves copying some logic (not to display the layout in XHR) to your local app that it would really be better to leave living in BL core (in case it changes in the future).

So that's my basic impetus, to make that use case work as simply as possible for newbies.