[D7] Form placeholder

Hi rafal,

Some things i found:
code sniffer found some warnings and errors http://ventral.org/pareview/httpgitdrupalorgsandboxrafalenden1782048git

Please add an README.txt for instructions.

Delete your master branch and add a 7.x-1.x branch. Also read this article.

Hi rafal,

manual review:
- there is an empty form_placeholder folder inside your module
- form_placeholder.js is missing a ; at line 23

suggestion (just a personal feeling)
- make the pattern text fields a text area, so you can enter different forms each on a separate line; would make it more readable if you have long selectors for different forms;

Regards

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.

Few minor suggestions (7.x-1.x branch) from a manual review:

Line 35: user_access is the default access callback so it's not necessary to explicitly set it.
Line 79, 86: Do we really need to set #maxlength ?
Line 122: The website URL seems to be broken.

Nice work by the way. Also, I'd love if it would be possible to use some Form API property to force the form title to be a placeholder (Rather than depending on HTML5 attributes).
For an example, if I set #jquery_placeholder => TRUE, , then this module magically hides the title (without #title_display => invisible) and sets the placeholder ?

Hi, my manual review:

I don't know it is best practic, but i think, you may include form_placeholder.js in form_placeholder.info
All other seems good. I advise you use Review bonus system.

Good luck.

Hi rafal,
automated review:

FILE: /var/www/drupal-7-pareview/pareview_temp/lib/jquery.placeholder.js
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
71 | ERROR | There should be no white space after an opening "("
71 | ERROR | There should be no white space before a closing ")"
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/lib/jquery.placeholder.min.js
--------------------------------------------------------------------------------
FOUND 39 ERROR(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
8 | ERROR | Expected 1 space before "="; 0 found
8 | ERROR | Expected 1 space after "="; 0 found
8 | ERROR | Expected 1 space before "=="; 0 found
8 | ERROR | Expected 1 space after "=="; 0 found
8 | ERROR | Expected 1 space before "=="; 0 found
8 | ERROR | Expected 1 space after "=="; 0 found
8 | ERROR | Expected 1 space before "="; 0 found
8 | ERROR | Expected 1 space after "="; 0 found
8 | ERROR | Expected 1 space before "="; 0 found
8 | ERROR | Expected 1 space after "="; 0 found
8 | ERROR | Expected 1 space before "==="; 0 found
8 | ERROR | Expected 1 space after "==="; 0 found
8 | ERROR | Inline control structures are not allowed
8 | ERROR | Expected 1 space before "="; 0 found
8 | ERROR | Expected 1 space after "="; 0 found
9 | ERROR | Expected 1 space before "="; 0 found
9 | ERROR | Expected 1 space after "="; 0 found
9 | ERROR | Expected 1 space before "=="; 0 found
9 | ERROR | Expected 1 space after "=="; 0 found
9 | ERROR | Expected 1 space before "="; 0 found
9 | ERROR | Expected 1 space after "="; 0 found
9 | ERROR | Expected 1 space before "="; 0 found
9 | ERROR | Expected 1 space after "="; 0 found
9 | ERROR | Expected 1 space before "="; 0 found
9 | ERROR | Expected 1 space after "="; 0 found
10 | ERROR | Expected 1 space before "="; 0 found
10 | ERROR | Expected 1 space after "="; 0 found
10 | ERROR | Expected 1 space before "="; 0 found
10 | ERROR | Expected 1 space after "="; 0 found
10 | ERROR | Expected 1 space before "="; 0 found
10 | ERROR | Expected 1 space after "="; 0 found
10 | ERROR | Expected 1 space before "="; 0 found
10 | ERROR | Expected 1 space after "="; 0 found
10 | ERROR | Expected 1 space before "="; 0 found
10 | ERROR | Expected 1 space after "="; 0 found
11 | ERROR | Expected 1 space before "=="; 0 found
11 | ERROR | Expected 1 space after "=="; 0 found
11 | ERROR | Expected 1 space before "="; 0 found
11 | ERROR | Expected 1 space after "="; 0 found
--------------------------------------------------------------------------------

Manual review:
the code seems to be fine.
However, to find the nit, I should:
1) improve the minimalist README.txt
2) move form_placeholder_admin_settings() to a separated file (I think)

Cheers

1. Why are you adding placeholder element in JavaScript rather than processing HTML at the backend? Its a nice small module, but I think that moving transformation to one of the form hooks would be a better way of doing this. See http://drupal.org/project/placeholder.

2. I don't think you should provide jquery.placeholder.js with the module. Require users to add it to the library folder and get it from there. Here is quote from documentation of Placeholder module:

INSTALLATION

- Install libraries module (dependency).
- Download jQuery-Placeholder library from https://github.com/danielstocks/jQuery-Placeholder
- Place this library in sites/[all/sitename/default]/libraries/placeholder
so the jquery.placeholder.js is located at sites/[all/sitename/default]/libraries/placeholder/jquery.placeholder.js
or glone directly; 'git clone --recursive git://github.com/danielstocks/jQuery-Placeholder.git placeholder'
in your libraries folder.

3. If you for some reason insist on JavaScript to alter HTML, I suggest using switch for selection control instead of else if.

Good job rafal. The form API changes are really nice.

Manual review:
Code: Nothing found.
Functionality: As expected.

No issues found in manual review and issues mentioned by all the previous reviewers seems to be fixed very well.

All works fine and the drupal coding conventions inclusive usage of integrated drupal functions were well implemented, so the application status should be "reviewed & tested by the community"

Could you point out the differences to similar modules (i.e. Placeholder and Compact forms)?
As we prefer collaboration over competition we want to prevent having duplicating modules on drupal.org. If the differences between your module is not too fundamental for patching an existing one, we would love to see you joining forces and concentrate all power on enhancing one module.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and we will take a look at your project right away.

Thanks for the clarification.
Placeholder module targets the Form API so end users who are not willing to form_alter or create forms from the scratch will find this Form Placeholder module easy to use.

One suggestion is that module names are a little confusing, don't you think? JQuery Form Placeholders or such name will make the purpose clear and distinct from the others I guess.

Wouldn't it be better then to contribute the difference to Placeholder as a patch instead of adding just another module that "basically does the same but adds feature X" (simply spoken)?

Btw.: please don't change status back to "reviewed and tested" as the applicant by yourself.

Hello Rafal!

Please correct the errors from automated report: http://ventral.org/pareview/httpgitdrupalorgsandboxrafalenden1782048git (add spaces).

Best regards,
Peter

minor coding standard errors are not application blockers, please do a manual review.

You're loading quite a bit of code in form_placeholder_init() - this will be executed on every Drupal page request. Please consider only loading these files when needed (when the user is on a form page).

lib/jquery.placeholder.js (and min.js) appear to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you´re not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

It seems jquery.placeholder.js is still being maintained at https://github.com/mathiasbynens/jquery-placeholder. Can you use that with libraries api and include installation instructions in your README?

Setting to needs work for the library issue, otherwise this will be RTBC from me, looks very useful.

----
Top Shelf Modules - Enterprise modules from the community for the community.

+1 for the libraries API usage. But that library is GPL'd so no licensing issues with drupal repos I think.

Code review looks good and the module seems to work as advertised.

I wasn't able to get this to work with webform module forms, though I think this would be more of a feature request than a bug. Still, webform is one of the top installed modules in Drupal so it might be worth adding support for it.

Please set applications to "needs work" only if there's a major issue or response required from the contributor.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Looks pretty good on review, thanks for cleaning up those issues.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Thanks for your contribution, rafal.enden! You are now a vetted Git user. You can promote this to a full project and also create new projects as either a sandbox or a full project.

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 (here you get to ask fans of your project to help you!)

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.

And thanks to the awesome reviewers, including hardcoding, Ayesh, pagolo, Caseledde, bulat, kriboogh, and the unstoppable kscheirer!