features support ?

I'm interested in this as well. Thoughts anyone?

Not yet. Patches welcome :-)

The best way to accomplish this would be to integrate block_class with the boxes module, which is compatible with features.

The best way is to add features support in this module. No need for another (boxes) module. Addding features support can't be that hard... Block Class has its own db table, so it can make clean exports.

Together with rudiedirkx we've created the features implementation. You just need to drop this file into your block class module folder and it works.

Rudie wants to explore if he can load the classes from code instead of from code. So he might improve on this later.

Awesome. I had to fix the patch a bit, but I've tried exporting and reverting and it's working for me.
Thanks Johnny vd Laar & rudiedirkx!

The patch in #5 wasn't applying (error: malformed patch) because it's missing a newline at the end of the file. Also, the patch should be made relative to the module directory, not your site root.

I also added a @file description comment to block_class.features.inc (to follow docs conventions,) but haven't changed any code.

Excellent @jeffschuler!

Any reviewers? (I don't think I'm supposed to review my own handiwork.)

Quick review:

+++ b/block_class.features.inc
@@ -0,0 +1,94 @@
+
+/**
+ * Implements hook_features_api().
+ */
+function block_class_features_api() {
+  return array(
+    'block_class' => array(
+      'name' => t('Block class'),
+      'feature_source' => TRUE,
+      'default_hook' => 'block_class_features_default_class',
+      'file' => drupal_get_path('module', 'block_class') . '/block_class.features.inc',
+    ),
+  );
+}
+

This needs to move into block_class.module.

+++ b/block_class.features.inc
@@ -0,0 +1,94 @@
+  $theme = variable_get('theme_default', 'bartik');

Why default theme only? Its not possible to export classes for blocks in admin themes with this setting.

+++ b/block_class.features.inc
@@ -0,0 +1,94 @@
+  $blocks = db_query("
+    SELECT CONCAT(b.module, ':', b.delta) FROM {block} b INNER JOIN {block_class} bc ON bc.module = b.module AND bc.delta = b.delta WHERE b.theme = :theme

Line-break before "SELECT".

Why not db_select?
Its better than building the query string manually.

+++ b/block_class.features.inc
@@ -0,0 +1,94 @@
+  $theme = variable_get('theme_default', 'bartik');

Unused.

I think we can drop the filter on just the default theme.

Should we allow all themes or just the default theme and the admin theme?

Hm, it would be cool if one could export block classes for each theme individually. So you can export settings for seven and rubik for example, but do not include settings for other (enabled) themes.

Nodeblock doesn't store it's settings per theme, just per block so I don't think this is possible with the current db schema.

Attached is a new patch with the advice of stBorchert included.

Patch from #13 doesn't work for me as the actual block classes weren't exported. I'd recommend the following changes:

diff --git a/src/sites/all/modules/contrib/block_class/block_class.features.inc b/src/sites/all/modules/contrib/block_class/block_class.features.inc
index c886c91..4142f98 100644
--- a/src/sites/all/modules/contrib/block_class/block_class.features.inc
+++ b/src/sites/all/modules/contrib/block_class/block_class.features.inc
@@ -33,15 +33,16 @@ function block_class_features_export($data, &$export, $module_name = '') {
  */
 function block_class_features_export_render($module, $data) {
   $query = db_select('block', 'b');
-  $query->addExpression("CONCAT(b.module, ':', b.delta)");
   $query->join('block_class', 'bc', 'bc.module = b.module AND bc.delta = b.delta');
-  $classes = $query->execute()->fetchAllKeyed(0, 0);
+  $query->addExpression("CONCAT(b.module, ':', b.delta)", 'id');
+  $query->addField('bc', 'css_class');
+  $classes = $query->execute()->fetchAllAssoc('id');

   $code = array();
   foreach ($data as $id) {
     if (isset($classes[$id])) {
       list($module, $delta) = explode(':', $id);
-      $css_classes = isset($classes[$id]) ? $classes[$id] : NULL;
+      $css_classes = isset($classes[$id]->css_class) ? $classes[$id]->css_class : NULL;
       $code[] = compact('module', 'delta', 'css_classes');
     }
   }

Thoughts ?

Yes you are right. Attached is a new (better tested) patch.

@Johnny vd Laar
great patch in #15, works good to export and deploy. activated feature with a block and a block class on new site, class was there as expected.

Export itself now is working fine but when (re-)creating a feature, the component listing for "Block class" displays all available block class settings, even those that are already exported by other features.

#17 I saw that today too. I think that should be Features' responsibility though. Only Features knows which features are enabled and which components they contain. Features also keeps a cache where that should be stored. I don't know why it doesn't...

Nope. It's block_class' fault. hook_block_class_features_default_class shouldn't return a numeric array.

I fixed it in this patch. Was easy enough. Doesn't invalidate (but does override!) existing features with block classes. You have to recreate your existing features to have them not overridden.

#15 isn't good enough. It doesn't use the features defaults hook correctly (like a true Features user would expect). The patch in #20 does. I'll recreate it tomorrow.

@rudiedirkx
Thanks, that's great. When you post it can you mention which version you created the patch against? Current stable or dev.

Rerolled it. Against dev (292601fc0adbcbdd3175a2316047f878259e65d9).

Let's include it already. 13 months is long enough.

PS. I think it's the exact same patch as #20... Don't know why it wouldn't apply.

patch in #24 applies and works as advertised. Thanks for your work rudiedirkx. One more tester and then set to rtbc?

Hello guys nice work,

is it possible to merge this patch with the "features extra" module? It would be nice to work with an unique id to get the settings of a block.

@komochti: could you please create a new feature request issue and report here just how you find the last patch working?

Merge with what in the Features Extra module? And why? Would that make the Features Extra module a requirement? That would be very silly. Link?

Blocks already have unique id's: module + delta. UUID's aren't relevant, are they?

@rudiedirkx: I saw what he means. With custom blocks you only get the block id which can be different across several sites when exporting blocks with Features. Then someone exports a block class for say block 4 and it gets applied to a different block on my site because on my site block 4 is a different one. Features Extra uses a machine name to export blocks, so I'm guessing that's what he's asking (but should be in a different issue). Unfortunately UUIDs, at least from UUID module, only work for entities. From what I saw today there's no existing solution to this problem.

@aschiwi The features patch we (Johhny and I) made works with machine names too: module + delta. Custom blocks are content, so they have a 'contentual' delta. (Not the `bid` but some serial int.)

Since most - by far- blocks are blocks made by other modules (which means meaningful delta's) I don't think it's a problem... It'd be nice if it worked ofcourse, but it'd be stupid to require an additional module (Features Extra) to create a tiny feature (block class features) in a module (block class) that doesn't need that additional module 95% of the cases (good delta's).

How would it work, though? I'm curious. What does Features Extra add? A table with UUIDs? It might be possible to make it an 'optional dependency'. (I know that's not a thing.)

@aschiwi I think block_class features and Features extra would play well together. It requires tiny changes in this patch and then it'd check module_exists('features_extra').

I'm not making the tiny changes though =) I'm very content with the current state.

@rudiedirkx yeah let somebody create a new issue for that and set this to rtbc to get it in :) thanks for your work everyone!

New patch with integration of Features Extra.. If there is a machine_name for a block it will be used. Otherwise the blocks delta is used.

Unfortunately this leads to a bug in Features Extra itself. Patch is here: #1762678: Trying to get property of non-object ... fe_block.module line 443.

Never mind, the patch makes false assumptions (I thought $delta would match the block id).
Needs complete rethinking and rework ...

$delta isn't enough. You need a module - delta combination but I would force the user the enter a machine name.

alright people let's get the patch from #24 in. new issue for machine name integration please.

Committed #24, thanks guys!

Great news!

Here is the commit for reference - http://drupal.org/commitlog/commit/5828/f34e5a1786a6887edaf5789c032ff1aa...

thanks everyone!

Thanks! But is this working for custom blocks (boxes) created from admin UI?

(I have some content boxes and currently I export/import them by Feature Extra module. Of course block class is missing after import.)

OK - I've tested it and it does not work for boxes. Reason was explained by #29.

Nice patch! Works as expected.

2.1 doesn't have it, and I don't like fe_block, so here's the same method for 2.1.

Ah, the functionality added in this issue was removed from block_class, and provided via the features_extra module. See #2110593: Reintroduce Features integration in 2.x branch.