Reintroduce Features integration in 2.x branch

Hi @idebr,

Thank you very much for posting this feature request, submitting a patch and for your interest in the Block Class module.
I am certainly very grateful for your positive feedback and greatly value your support for the work that has been recently carried on the 7.x-2.x branch.

However, it seems the features integration was deleted in the initial commit of branch 7.x-2.0

Yes, indeed and I thought you might perhaps be interested in taking a closer look at the documentation for the 7.x-2.x branch in #1868586-2: Alternate structural approach: modifying the block table?, in particular:

3 - Other changes (block_class.info, block_class.features.inc)

a. Cleanup (on the way):
While modifying the module, I allowed myself to change/adapt a few inline comments in all the files.
Cleaned up block_class.info file.

b. Features integration not needed anymore (See #1230234: features support ?):
This is another good side effect of staying closer to existing structural objects, in particular Drupal blocks.
Since the css_class is now implemented as a block property, then I had the pleasant surprise of finding it was also correctly exported by Features Extra's great fe_blocks module which would remove any need for any additional code in this module.
Therefore, any code related with #1230234: features support ? was simply removed in this implementation.
(Removed implementation of hook_features_api in block_class.module and the entire file block_class.features.inc)

I would like to specify this has been thoroughly tested for exporting, enabling and reverting any css_class settings exported through fe_blocks features.

In short: Exporting the entire Block's configuration along with its classes should now go through Features/Features Extra out-of-the-box.

Module's code being closer aligned with Drupal's objects, it is now able to leverage Export features from other modules out-of-the-box, without any additional logic: less code, less logic, but still supports all the features of the 7.x-1.x branch, while getting better performance results.

Since I wouldn't necessarily see any more work that would need to be done in the module in this area, I allowed myself to mark this issue as fixed for now, but feel free to re-open it, or post a new ticket, at any time if you have any further objections with this issue or the changes made to the logic to integrate with Features (we would surely be happy to hear your feedback).

Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on this comment or the ticket in general, I would be glad to provide more information or explain in more details.

Thanks again to everyone for your testing, reviews, feedback, great engagement and comments on this issue.
Cheers!

Hi @idebr,

Thanks a lot for your kind, prompt feedback and for revising the patch.

Please allow me to reply to your comments directly:

There is no upgrade path for Block Class features defined in version 1.x to 2.x. This patch makes sure block classes that were defined in Features in 1.x still work.

Very good point! I completely overlooked this issue when creating the new releases for the 7.x-2.x branch.
This is probably the biggest issue here and I assume it would have to be further discussed, in particular: whether compatibility should be kept or not (given that 7.x-2.x is a major release - see more at the bottom of this comment).

Features Extra is by no means perfect and opens up a new set of possible feature conflicts due to the exports per theme.

I haven't tested the patch, but after looking at the code, unless I missed something, it doesn't seem to export any particular information related with the theme. Therefore, I wouldn't necessarily be convinced this patch would resolve the conflict issues mentioned, related with exports per theme.
On top of that, one could argue that if the Features Extra module "is not perfect", efforts should then perhaps be made on improving it rather than working on Block Class.

Features Extra does not export all block classes. Most notably, there is no support for Menu Blocks and there is no sign of support in the near future as the issue is marked postponed: #1947816: Add support for the Menu Block module

I have already extensively detailed how Menu Blocks' configurations could be exported with corresponding Block classes at #1894280: Exporting Menu Blocks with Features Extra and Strongarm (with test features exports and code dumps), with the current version (menu_block-7.x-2.x).
Additionally, extensive work has already been done as an attempt to improve exporting Menu Blocks' configurations at #693302: Add API for exportable menu_blocks / Features integration (where there is already a pretty advanced patch, that has received several rounds of tests/revisions, although it has been stalling for a while - needs help).
So the solution for exporting Menu Blocks is maybe not going to come from Features Extra, but rather from Menu Block (work in progress, needs help) itself or Features/Strongarm (currently works).

The install base of Features Extra is a lot smaller than the Features module: 11602 vs 144663 sites. This suggests the additional functionality offered by the Features Extra module is not interesting for 92% of the Features users.

One could argue that not enough modules have been leveraging Features Extra's API and features and therefore, it is still taking time for users/developers to change their habits and move towards using Features Extra on a more common basis.
Additionally, I would assume that if enough interest could be shown for certain features of the Features Extra module, it could be further discussed or considered to potentially merge these requested features into the Features module.
In other words, moving fe_block to Features and allowing users to export Block settings configurations out-of-the-box.

Since Features Extra offers features integration for Block Class, there is no need for Block Class to do so.
Features integration should still be available for users without Features Extra enabled through block_class.features.inc through this hook:
...

Would that be really necessary? If possible, could we try to weigh out a little bit the Pros and Cons here?

I'm not particularly convinced that offering the possibility to users to only export Block's CSS classes (from what I understood from the patch), would be a substantial improvement.
Wouldn't most users be actually interested in exporting the "whole thing" with the full Block configuration settings (along with the CSS classes)?

Furthermore, I would personally assume there would be a non negligeable cost in terms of code maintenance, since we would be maintaining/updating the code based on an API from an external module (that actually changes quite often), which is not the case currently, where we completely discharge ourselves from any maintenance work in this area (no work on exportables at all).
Not to mention support or update issues and compatibility between different versions of Features and Block Class .... it sounds to me like it would be breaking a lot of the "piece of mind" we worked so hard to obtain by re-thinking module's architecture from scratch.

Wouldn't it sound more logical/reasonable to try to keep all Block configuration settings related code export logic and issues in the same place, grouped all together?
We could all put our efforts in improving the Features Extra module and working on getting this particular piece of code (fe_block) of interest into the Features module (maybe some day?), instead of creating scattered pieces of integration code with Features that would all need to be tested and maintained independently.

One could return the question: Why can't Features export Block configuration settings out-of-the-box?
(I would personally be one of those interested in this feature)
 

To sum up:
All that being said, I assume you would have understood I would have great doubts about the necessity of adding this feature to the new branch of the module (block_class-7.x-2.x) and therefore wouldn't be particularly favorable to seeing this committed.

Instead, as far as compatibility/regression issues are concerned, for users upgrading from the 7.x-1.x branch, I would rather be in favor of updating the documentation, project page and letting users know compatibility with Features has changed in favor of the Features Extra/fe_block module: All the Features that were previously using the Block Class Features export functions (with a dependency to block_class), would have to be manually updated and the Features re-created with fe_block.
Rather than allowing users to stick with what some could consider as an "improper" or "incomplete" way/solution for the handling of Block Class exports.

If users/developers are not ready to consider re-creating all their features with Block Class dependencies, then they should most likely stick with the 7.x-1.x branch.

Again, I would certainly be open to discussion on this, but I wouldn't really consider this as a regression.
The exact same results could be achieved, with the only difference that we have added a dependency to fe_block for this particular function, which is a specialized solution for doing something probably better than what Block Class has been trying to do before.

I hope I was able to provide reasonable answers to your concerns and that it might give you another idea of how the problem could be approached from a different perspective.
I would greatly appreciate your feedback, comments, questions, concerns, suggestions or issues on any aspects mentioned in this comment or this ticket in general.

Thanks again very much for your feedback, interest and in advance for your answers, comments or questions.
Cheers!

Hi @idebr,

Thanks again very much for your clear reply and re-submitting a patch.

Please allow me to reply to your comments directly:

Keep backwards compatibility with the 1.x branch.

Backwards compatibility is a topic that is greatly argued in the community and assumed not a given in Drupal for Major releases in general.
In particular, it doesn't seem this would be a rule that all modules would have to necessarily respect and I kindly invite you to take a closer look at Backward compatibility for data, not code, where many reasons Why ignore backward compatibility? are listed. (I personally, currently share a lot with them)

Keep block_class modular. Block class logic and its integration with other modules should be handled in the module itself. This is how integration with other modules typically work, for example integrations with views or drush

Ok, sure, I can definitely understand integration code is important. But block_class-7.x-1.x requires the Features module for its integration. The 7.x-2.x branch also has some code written (not API specific, but architecture changes allowing compatibility out of the box) for integrating with FE Block and not Features anymore.
I'm afraid I can't really see what the problem could be: 7.x-1.x it needs Features to export and 7.x-2.x it needs FE Block, with the same results that could be achieved (exporting Block's CSS classes to Features).

By leaving features integration to Features Extra, developers who are maintaining a site now have an additional module to maintain.

Yes, sure that makes another module to depend on and add/maintain to site's installation, but maybe if some day FE Block could be integrated to Features, that wouldn't be a problem anymore. It would seem this problem wouldn't come from the fact that Block Class would have to depend on FE Block for exports, but rather that FE Block would depend on Features. Ideally, Features could provide Block settings exports out-of-the-box.
I might be wrong, but isn't modularity about using different modules that specialize in different specific applications? Trying to break features in smaller modular chunks re-usable by other modules?

The use of block visibility settings is actively discouraged due to performance reasons by webchick, one of the core maintainers. Instead of using the block visibility settings, most developers choose to use a contrib module to display blocks, such as the context module, instead. This leaves very little use for the fe_block module for those developers. The difference in install base between fe_block and context is evident.

hum..... you are not a big fan of Features Extra, are you? :-)

Thank you very much for indicating this very interesting post about Drupal Performance and Block Visibility, but please correct me if I'm wrong, from what I understood in this post and I might perhaps have missed some related links, but it doesn't seem to be discussing performance comparison of the Core Block Visibility settings with other contrib modules such as Context.
This Blog post explains that Blocks would render on all pages, whether the regions are to be displayed in template's markup or not.
Besides, the post ends on suggesting to set Blocks Visibility settings to prevent them from loading on pages with templates that wouldn't render the display region (For example, only display certain block for <front>).
It doesn't appear to me it would be really relevant or related with the current matter, or as any evidence that would compel "most developers" to use other Contrib modules.

Indeed, the number of installations seems to be very different, but with only this information, how could any firm correlation be really established to justify the fact that using Block visibility settings wouldn't be recommended, or any more precise deductions about the approaches preferred by developers. I would assume there might be plenty of reasons or factors for which users/developers might prefer to use Core Block Visibility settings, Context, FE Block or Features, etc...
Unless there would be more factual evidence (surveys, usability studies, more precise/specific stats, etc...), it would seem to be difficult to extrapolate any further than that on any of the reasons why there would be such a difference.
I would certainly be very interested in learning more on this.

I personally believe that if there were more users of FE Block, there would be more testing, more efforts combined/made on improving it and the community might have more leverage in discussing potentially merging it into the Features module down the road, which I would think would be a substantial improvement.
 

If we were to follow the same kind of reasoning based on numbers, one could very well return the question and say that if the 7.x-1.x branch's integration with the Features module was so important, widely used and popular, then perhaps this issue would have been raised much earlier and not after 7 months with already almost 15,000 sites declared using the 7.x-2.x branch (just a few thousands away from 7.x-1.x).
Would we be making a mistake if we were to extrapolate on the fact that few users would actually really be using 7.x-1.x's Features integration and that most of them would in fact support the changes in the 7.x-2.x?
 

Block Class should handle its own integration with other modules to keep code modularity.

It already does and integrates out-of-the-box with FE Block. Integration doesn't always have to go through implementation of API hooks, it could also be architectural changes bringing in compatibility.

Backwards compatibility with previous version is very important. In this case there are no significant gains by removing backwards compatibility, other than maintaining a little less code.

For major releases and new branches, backwards compatibility is not always a given in Drupal. Those who would like to update from 7.x-1.x would have to manually update related features and add a dependency to FE Block.
This would have to be documented in the README.TXT and project's page.
As for the "little less code", I'm not certain it would be accurate to be minimized this way, since it would also include all the work on documentation, support, testing, etc... which takes quite some time, efforts and resources.

Lastly, I would like to add up to that my scepticism on providing different methods for users to achieve the same results. I am greatly afraid there would be even more conflicts, compatibility issues and update issues that could potentially add a greater burden to the maintenance or future evolution of the module (for example a site with features that would use both export methods, which would override each other...).
 

Once again, I would like to renew my greatest concerns about this feature and would surely be very interested in hearing what others would have to say about these different approaches, opinions and perspectives of how Block Class should integrate with Features.

Please let me know if I missed, misunderstood or overlooked any elements in your last reply, it would certainly be very helpful to allow the discussion to move forward.
I would greatly appreciate to hear your questions, comments, issues, concerns, ideas or suggestions on any aspects discussed in this comment or this ticket in general.

Thanks in advance to all for your feedbacks, questions, comments and ideas.
Cheers!

8: block_class-features-integration-2x_2110593-8.patch queued for re-testing.

Changing this to Feature request.

No feedback on this issue for more than 2 years..
I would assume an appropriate solution should have already been found by ticket's author.
Additionally, no one else reported any issue that could be similar or related.

Therefore, I allowed myself to mark it as Closed (won't fix) for now, but feel free to re-open it, or post a new ticket, at any time if you have any further objections with the approach suggested in this ticket (we would surely be happy to hear your feedback).

Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on the suggested solution, this comment or this ticket in general, I would be glad to provide more information or explain in more details.

Special thanks to @idebr for raising the issue, testing/reporting, submitting code and documenting in great length the benefits of an integration with the Features module. It's all gonna pay off once we move to D8.
Many thanks to everyone for your great help, reviews, testing, reporting and participation in this module's issue tracker.
Cheers!