Make use of Context API instead of custom rules

Rules in Skinr are totally useful without the Context module and for that reason, I wouldn't consider fundamentally changing the way they work. The functionality mirrors the block visibility options, which works well for most situations.

People use Skinr and context together though, as far as I know, so maybe what you are trying to do is already possible? Maybe if you explain exactly what you are trying to do, we can determine whether or not you are getting the functionality wrong or not.

I haven't tested or used Context module yet, but read and heard a lot of good criticism about it. Will try to test it ASAP.

In case it turns out to be sane and cool, we should actively brainstorm and consider to replace Skinr's custom code with it. Less code to maintain, and offloading maintenance burden to dedicated projects, is always a good idea.

Note, however, that this issue/proposal might contradict what I proposed/we discussed just yesterday — removing "rules" as separate instance in Skinr and merging the functionality into the regular skin configuration/administration ("it's just about determining the target elements. D7 changes a lot in this regard, should actually be easy.")

Should it work with Context? Absolutely. But, I'm not replacing Skinr's custom code with code from ANY other module. What Skinr does it easy once you've use it. Skinr's UI is a lot of the problem you (sun) have faced, but you also came in with a preconceived notion of how Skinr should work. We've been looking to branch off with our own UI because it has been pure hell dealing with supporting Panels, since well over a year ago. This would mean no separate UI for rules. The UI has been a problem since day one. We know this. Fixing it is part of the goal with the work we are doing now.

Less code to maintain, and offloading maintenance burden to dedicated projects, is always a good idea.

If that was the case then we should have used CTools for includes. Relying on other modules is great for certain cases, but in this case I think it's a bad idea. It's just going to add another layer of complexity to something that should be VERY simple. If something like this is going to be entertained, it's not going to be until the next major version as far as I'm concerned.

well, as mentioned, I haven't used Context yet, but I had a quick look at its code, and also know that a couple of modules already implement contexts. Please understand that I'm trying to evaluate this from a technical architecture perspective -- figuring out contexts is a pretty complex problem space. While we currently pretend to support a page context, the world of contexts is actually much larger - domain/site, language, authenticated/anonymous, user roles, etc.pp -- for this, you need a dedicated system that is able to figure out and provide the contexts; something we're not going to (re-)invent within Skinr.

Note that I'm talking about Context, not Context UI. It's a pure API module.

Context module is an API that tries to introduce a proper notion of contexts; which you basically know of Block module's block configuration form already. However, the block module contains all kind of shit and custom logic to figure out those contexts - they are not standardized in any way; Block module merely says, "oh, hey, we have user roles, let's provide an option to show this block depending on user roles."

The underlying, true context is a "user role". Forget about Block module and its UI and everything else. Just imagine that you have a function that you call:

$contexts = context_get_contexts();

And in case you happen to have a "user role" context plugin, then that variable will contain something along the lines of:

$contexts['user_role'][1] = TRUE;
$contexts['user_role'][2] = FALSE;

What you do with this context information is your "problem". The Context API does nothing else than to provide a pluggable system to define and determine the context of a page request.
So by leveraging the API, Skinr would be able to ask:

"I have this skin here, does it match the current context, so we can apply it?"

But no UI consequences or whatsoever, that's left to your specific application.
The difference to block visibility settings is that they are very limited and not extensible.

I still think this is just opening up a can of worms. I don't see how people wanting to use Context with Skinr will not want to use their UI. I'd have to see this in action, and it would have to be inline with the goals we have for the streamlining UI for me to be on board with it.

For what it's worth, I'm in favor of optional support for the Context module. It would be great to extend the functionality that Skinr provides with the more extensive options available through Context, but this doesn't need to come at the cost of requiring the module as a dependency which I don't believe we should do.

Skinr could still provide it's standalone basic rules (context) options as to not require the Context module. From there it could optionally support the Context module. That integration has two options:

1. Support the Context api to integrate with Skinr's own rules logic/functionality.
2. Add general Context module integration whereby users instead work with the Context module's UI. This means that a user could set a skin to be enabled for a block, with conditions, and without using Skinr's own UI for that.

I'm in favor of #2 though I haven't looked in detail at what #1 would take to accomplish. Going with #2 is preferred because it should be less Context integration code to maintain.

I agree with @ChrisBryant, +1 for supporting Context without creating a dependency. Their rules engine is more robust and supported, but aside from that, site builders using Context will appreciate having a single rules UI. I count myself in that category. Selecting which blocks are to appear on a particular page for a set of users, for example, is something Context does very well. Theming those blocks by applying skins at the same time inside of the Context UI makes sense. I'd be happy to help out with that.

Although I don't think it's actually under discussion to remove the Skinr rules UI, I want to second its value -- there are many site administrators who feel perfectly comfortable applying skins, but would never touch a Context. There's a fear-factor difference between menu items under "Appearance" vs. under "Structure", and one of Skinr's biggest appeals over something like Sweaver is the assurance that you can't break stuff. Rules seem like a stretch for that user base, but perhaps not. Even on the developer side, there's sometimes a "context vs. panels" mentality, so I wonder if people using Panels would be confused by a Context module dependency.

re @sun's brainstorm to overhaul the rules backend, I've used the Context API before, but I'm not quite sure what the convincing argument for reworking everything might be, given that everyone involved with Skinr is stretched for time, and what you've developed so far is working. That said, settings/getting a context using their API is easy once the fuzzy terminology is understood, and there's the added (free) bonus of the Context Inspector for the Admin module/toolbar. It'd provide transparency over which Contexts are loaded on which pages, with a listing of their respective Reactions. Since a skin would be recast as a Reaction, it'd be listed on in the toolbar even if its CSS or JS is not applying for some reason -- eg, due to a missing asset like a CSS file, or because the targeting is off. Though I'm not sure how popular the Admin toolbar is for Drupal 7.

I've been digging around Context API, and it looks like due to their caching the overhead (after caching occurs) is minimal at an additional 4/5 queries.

A potentially large benefit to using Context API is that we would no longer need to have individual skin configs for each theme, but could use context to determine which theme(s) a skin config applies to.

A downside is that Context depends on CTools... an additional dependency. We would also have to figure out a UI that works for us.

UI issues:

1. Allowing multiple skin configs per element means either of 2 options: a) have a (potentially large) list of 'edit skin x' links in the contextual menu, or b) have 1 'edit skins' link and then have a follow-up page allowing you to select which skin config (or context) you want to edit... neither option seem ideal; a bloated contextual menu list, or a multi-step process to edit a skin. Suggestions welcome.
2. Once you get to editing the skin config (or context), we don't want the UI cluttered with all Context API's reactions; we'll be limiting it to only Skinr's reaction (which we'd have to build; it would essentially be our current Rule edit form at admin/appearance/skinr/rules/edit/%, where the visibility settings are replaced by Context UI's conditions editor.
3. We might want to consider a default with 'Sitewide context' condition is 'Always active' if no additional conditions are set.

Thanks for reviewing the Context API integration option @moonray. As @jacine and @aquariumtap have noted, we should definitely approach it from the standpoint of being a separate, non required dependency/module. With that being the case then it appears we'll still need individual skin configs for each theme, regardless.

Regarding #1, I assume this would only be a concern when having the non dependent Context API integration enabled. I'd go with the multistep approach if there is the potential for more than 5-6 edit skin links appearing in the contextual.

@ChrisBryant: 1. is already a problem with Rules UI, at present. Our current 'solution' is less than ideal if we have many rules applied to a page or region.

@jacine and I have been discussing integrating with Context API (not the UI!), and there seem to be quite some benefits to requiring it. I feel that having an optional dependency will not allow us to take full advantage of what it has to offer.

If it being a required dependency allows for tighter integration, saves time, and helps everyone out then I'm all for it. :-)

+1 for skinr + Context API

Integrating the Context API would allow for very powerful and flexible system to determine how and when skins should apply. Personally, I feel that the dependency on CTools or Context API is a moot point. Currently you are (re)inventing and (re)developing several systems that have already been invented and developed, and are already used in a huge number of popular modules. cTools is used by so many modules, that honestly, the more people that use it the better it is going to get. I don't think that the Context API has such a wide userbase, but it is a pretty well developed module from what I know. Why spend the time developing and maintaining those things within this module. Surely it would be better to spend the time that you would have spent maintaining Skinr's rules engine on helping to fix any problems with the context API - which already provides a much more powerful way of doing this stuff.

I'm working on this feature.

A breakdown of how it's going to be executed:

• Rules will be removed from Skinr, and page (html) and region hooks will be handled like any other hooks in Skinr; i.e. 1 set of skin settings per hook instead of the multiple that rules provided.
• A skinr_context module will be added that extends Skinr to allow multiple skin settings per hook (like rules previously did) with the functionality using context API at its core. Without this module enabled Skinr will work perfectly as described above.

One thing I've come across that I could use some feedback on is this:
Should we have an additional skinr_context_ui module (like we have a skinr_ui module) to enforce the required use of context_ui? Context_ui is required to provide theme function/templates and js to make the elements borrowed from context UI behave similarly as in the original module. Or should we instead duplicate that code (which is a fair amount) in order to not have to depend on context_ui?

I'm personally leaning towards the first of the two options I'm presenting. Of course, if you have additional options available I'll be glad to hear them.

Should we have an additional skinr_context_ui module (like we have a skinr_ui module) to enforce the required use of context_ui? Context_ui is required to provide theme function/templates and js to make the elements borrowed from context UI behave similarly as in the original module.

IMO, this would be better than duplicating and then having to maintain that code. I think it's fine given what your plans are. :)

And we finally have a patch!
What this patch does is:
• Removes rules functionality from skinr.
• Simplifies Skinr UI by removing legacy code related to js and module type form elements.
• Adds new skinr_context, skinr_context_ui module.

In the UI you'll notice that (when skinr_context_ui is enabled) editing skin settings for an element (i.e. "edit skin") now displays a list of groups. Each of these has separate "rules" which are set using context API, allowing you to enable a set of skin settings to just one page, or a section of the site, etc.
Also, the skin listing page at admin/structure/skinr now includes group info and filters.

Please test and review!
This is the last major API changing functionality for Skinr. After this we should be able to go to beta and then release!

I got tests back up and running... Wohoo!
But, it looks like d.o testbot doesn't like something in my skinr_context tests; it works locally, though. Let's try to include context module explicitly?

Third time's the charm. Missing dependencies entry... doh!

Since we're not getting any feedback, I've committed this patch to move things forward.
Please open a new ticket if you find any issues.