[D7] Term body class

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

A notice while checking the term body class occurs. Otherwise the module works perfectly.

Automated Review

[Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

No: Does not follow the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. * Below hook functions doesn't have the module's machine name.
   function taxonomy_field_body_class_form_alter(&$form, &$form_state, $form_id) {
function taxonomy_field_body_class_preprocess_html(&$variables) {

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Code too short

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

Automated Review

Passed

Manual Review

Individual user account

[Yes: Follows] the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

[Yes: Follows] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[No: Does not Follow] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements. / No: List of security issues identified.]

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. Possibly consider making the module bigger by providing more class related options, like defining your own body classes per content.
2. Works good on user pages too.
3. Consider writing readme docs as per https://www.drupal.org/node/2190239.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

I recently came across a case where I needed to add additionaly body classes on a node basis - also possible to do using context module tho. Thinking about it, making this module for just taxonomy is probably a nice way to package it.

Re the documentation, I actually meant that for this project page (not documentation), simply follow the guidelines, they display nicely and all the usual useful headings are there.

Was able to checkout via the clone command on the project page.

First, let me say great module. Well done. It worked exactly as intended. However, I did find an issue, as documented below in the review template.

Automated Review

$ phpcs --standard=Drupal *

    FILE: ...\htdocs\sites\all\modules\term_body_class\term_body_class.module
    ----------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but
    |       |     found "\r\n"
    ----------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------

    Time: 410ms; Memory: 4.25Mb

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the README Template.

Code long/complex enough for review

No: Does not follow the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. * CSS Class Name collisions
   

   ---

   * If a user tags content with a term that already exists as a system, module or theme's CSS class name, then the page will receive that class's styling. Only in instances where the CSS class declaration exists on it's own with no preceding elements, classes or IDs preceding it (unless that element is body), and no elements or IDs directly chained to it as part of the declaration (.thisclass, but not div.thatclass) (again, unless it is body.thaaaatclass).

   As a result, for example,

   I tagged a node with "overlay", hence the body got the class "overlay". In Bartik's style.css line 1576, there is a declaration ".overlay #header" to display: none;. Hence, for having tagged the node with overlay, I've lost the administration menu.

   There are a few ways I can think of to remedy this. You might consider prefixing any classes added by Term Body Class with a unique identifier, such as tbc-termnamehere, so as to avoid these collisions.

   You might implement a blacklist, and either provide a warning to users that their term name collides with a system CSS class, perhaps only prefix that specific term, allow overriding of that displayed term name with a custom term alias for the body class string that will be inserted (for example: mytermthatcollides {bodyclassversionofmyterm}.

   I would probably recommend the simple prefixing, though the other methods would be more likely to push your number of lines of code past 120! :)

2. Line 51 of the module: machinen is misspelled.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

In Sublime Text change line endings from Windows to Unix.

Hi remkor,

This is not a blocker though. Apart from this, are there any issues or it can be RTBC?

If you are using Windows the try this:
git config --global core.autocrlf false
This is not a blocker.

Automated Review

Pareview.sh has found One Git errors.
http://pareview.sh/pareview/httpsgitdrupalorgsandboxsghosh2409019git

Git errors: Git default branch is not set, see the documentation on setting a default branch.

Thanks,

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: follows

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. (*) term_body_class_preprocess_html() uses only 2 entity node & user. Try to make it generic. For example if any entity having a field like term_reference, It should add the body class on that entity view page.
2. (+) Use hook_form_form_id_alter() instead of using hook_form_alter().
3. (+) Try drupal provided function for generating class instead of using your own.(Line no: 78/79).
   Use direct function drupal_html_class('tbc-'. $machine_readable))

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

Changed Status.

Thanks for your review Bapi

The README guideline is not respected. No template was used. And it one was used, it would have allowed @SGhosh to clearly outline the module's limitations. In this case, the limitation/know issues would have been that the current version of the module only supports node and user entities.

@SGhosh, if you adjust your README file, that may solve the issue 1. pointed out by Bapi.

For 2. and 3., they need to be fixed IMHO.

Hi SGhosh,
Good point by Asiby. Please mention it on README file that it supports only 2 entities(node & user).
In the next version you can try to implement it.

Hey SGhosh,

Thanks for your efforts. I think its a very handy module.

• Module works as expected.
• Code looks clean.

Looks like RTBC to me.

Hi,

I reviewed this module and looks like a short module but giving a good feature, more helpful for theming. Code looks clean and fixed all the issues mentioned in previous comments. Couldn't see more application blockers.

@SGhosh, meanwhile as per review bonus program you can review three more other application from the queue to prioritize your own application.

Thanks,
ARUN AK

No objections for more than a month, so ...

I have promoted the project for you. Now you can create the release of the project.

Thanks for your contribution, SGhosh!

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.