[D7] Webformclass

Not very much to say about complexity: it's a simple task so the code is short. I could add some fancy settings to enable/disable the view-mode class and a few things more, but I don't find any good reason to add superfluous complexity or to make the code loger than it needs.

The use of enable/disable and not install/uninstall is intentional. Disabling the module and not removing the field will make the field still exists and not been handled by the module, so it's value will be in the output (displayed), wich is an undesired behaviour. For obvious reasons, as it have to be removed when disabled, it is added when enabled.

In hook_node_view_alter: Since the field is added to webform type nodes (only webform type nodes, and all of them) checking the existence of the field implicitly checks the node type as webform. Making both checkings redundan't.

No duplication: I didn't found any module that does this. I'll appreciate it if you can point me one. I do agree that this causes certain fragmentation and could (should) be a feature provided by a bigger module/project, so I've added 'feature request' issue in Webform. As soon as I receive an answer for the webform issue I'll update this one.

Thanks for your review.

I don't really agree with your oppinion about "unexpected behavior" and I thought that those rewiew where not about that kind of questions. Anyway I've modified the module and now it don't rely on a specific field. The enable/disable/install/uninstall hooks are gone as the field, and so any possible disagree about them. :)

I'll change the status to needs review if not accepted as "feature request" for Webform or in two weeks if I don't get an answer.

Thanks.

The question about install/enable/disable/uninstall is quite complex, but I consider that if the behavior is not what a user (also you) expect there are reasons to change it. My english is not very good and perhaps my answer didn't sound as I wanted. I agree that loosing the value when the module is disabled can be something unexpected by the user, but on the other hand the user don't expect data to be keept when a field is removed from the content type. The solution is to add a real field, not using the common text field, but that means new schema, validation, storage, more queries to BBDD, etc, wich I consider too much performance impact for a very simple action, so I didn't build the module that way. I did really appreciate your review and took your point of view about this question as a contribution to quality, so I made the changes.

Maybe there is a missunderstood about the configuration parameter. The setting (target field) is the field that will be used to get the classes to put in the form, not the class name. The classes will be the value of that field on each webform If as suggested the user set a single target field will be from that field. If not set a value for this field the classes will be all the values of the fields in the webform that uses the "Webform CSS Class" formatter. I've change the label in the config form to ensure there is no missundertood in the future with the setting.

About including this in webform: DanChadwick says that he don't see this included in webform ( https://www.drupal.org/node/2426015 ) so making a patch for webform doesn't makes very much sense for me. The pointed issue about machine keys & views don't seems really related to the theming/styling question focused by this module.

I've edited the issue to match the current behaviour of the module and changed the status to "Needs rewiew".

Thanks :)

Thanks for your comment k_zoltal.

Duplication: I do agree (really), and most people I know that use webforms module also agree about this, and many of them also agree that this feature should be a "must" for the webform module. Webform project should have it, or at least I wish (just me?) that such a feature should exist. The lack of this feature makes not only me but many others to have problems whenever I have to deal with a theme for a website with several form with "different styles" created by the maintainers after the initial release because there is no way to choose an style or css attribute if not using a custom module, and also get problems with sites where there are dev/stage/production sites that sinchonize/share/import nodes between them because those nodes will have (probably/eventually) different node ids (webform don't give any other way to build different css selectors for forms -I'm not saying fields- ).
If you check the previous comments you will find that fragmentation have beed tried to avoid. An issue was already open (and closed) in webform: https://www.drupal.org/node/2426015. I understand the maintainer answer "I don't see this being added to webform" as a quite clear "no" and that this feature have been rejected. The project about machine keys in his answer is not related to the simple fact of adding css classes to a webforms, but focussed in sharing content identifiers bettween server.
I'm not trying to compete with anyone, I don't want to and I don't need to. I just want to avoid the many problems that the lacking of this feature in Webforms produce so many times (too much times) to me and all of us that work closer to the frontend than to the backend. (And we also have to listen customers complains about drupal because it's not a 'normal' module, but your own/experimental/sandbox module)
As you said: 'please get back to us and set this back to "needs review" '. I'll do it now, since the try to get this feature in Webform module was already rejected more than two weeks ago.
Seems strange to me how difficult (too much!) is promoting a single usefull clear and simple project from "sandbox" to "full" when all the reviewables requisites have been complied and how easily many people release much simpler/shorter and faulty/useless modules with heavy problems of every kind (including fragmentation/duplication). Anyway thanks for sharing your point of view and for helping to 'kick' this issue forward adding a comment.

I'll be pleased to ammend this module in the (reasoned) way that anybody suggest if that could make it to be promoted to full project. I'm not finding too much feedback in that sense. Maybe others (you could help also in this way) should address an issue to webforms project claiming this feature or a feature like this one... or rewiew and accept this one.

Best regards,
Miguel.

Nothing more to say about fragmentation or competition vs collaboration.
Before review please check the already open issue in webform: https://www.drupal.org/node/2426015