Description :
This module creates a crossword field type and renders it as a crossword.

The field is a collection of two fields : 'word' and 'clue'. If added to a content type with unlimited number of values it will be rendered as a crossword with all the clues below.

Credits :
The crossword is generated with a javascript based on Rich East's code found on codepen.io

The project link :
https://www.drupal.org/sandbox/marabak/2430325

Git clone command :
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/marabak/2430325.git crossword_generator

CommentFileSizeAuthor
#13 crossword.png20.21 KBblanca.esqueda

Comments

nabil.sadki’s picture

Status: Needs review » Needs work

Automated review

Git errors:

Review of the 7.x-1.x branch (commit 69e7058):

  • Remove "version" from the ./crossword_generator.info file, it will be added by drupal.org packaging automatically.
  • ./crossword_generator.module: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming
    function crossword_field_process($element, $form_state, $complete_form) {
    
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
  • DrupalPractice has found some issues with your code, but could be false positives.
    
    FILE: /var/www/drupal-7-pareview/pareview_temp/crossword_generator.module
    -------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    -------------------------------------------------------------------------
     99 | WARNING | Unused variable $settings.
    -------------------------------------------------------------------------
    
    Time: 39ms; Memory: 4.5Mb
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.


FILE: /var/www/drupal-7-pareview/pareview_temp/crossword_generator.module
---------------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 3 LINES
---------------------------------------------------------------------------
  95 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
 133 | ERROR   | [x] Expected 1 space before "+"; 0 found
 133 | ERROR   | [x] Expected 1 space after "+"; 0 found
 172 | WARNING | [ ] Avoid backslash escaping in translatable strings when
     |         |     possible, use "" quotes instead
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/crossword_generator.install
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 33 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/crossword_generator.css
---------------------------------------------------------------------------
FOUND 66 ERRORS AFFECTING 65 LINES
---------------------------------------------------------------------------
   2 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
   3 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
   4 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
   5 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
   6 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
   7 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  11 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  15 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  19 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  20 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  24 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  27 | ERROR | [x] Multiple selectors should each be on a single line
  28 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  32 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  33 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  37 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  41 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  45 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  46 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  47 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  48 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  49 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  50 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  51 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  52 | ERROR | [x] Blank lines are not allowed in class definitions
  53 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  54 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  55 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  56 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  57 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  58 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  62 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  63 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  64 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  65 | ERROR | [x] Blank lines are not allowed in class definitions
  66 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  67 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  68 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  69 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  70 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  71 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  75 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  79 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  80 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  81 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  82 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  83 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  84 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  85 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  86 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  87 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  88 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  92 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  96 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  97 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  98 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  99 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 103 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 107 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 108 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 109 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 110 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 114 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 114 | ERROR | [x] Expected 1 space after colon in style definition; 0
     |       |     found
 118 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 119 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 66 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 4 LINES
------------------------------------------------------------------------
  3 | WARNING | [ ] Line exceeds 80 characters; contains 108 characters
 24 | WARNING | [ ] Line exceeds 80 characters; contains 113 characters
 26 | WARNING | [ ] Line exceeds 80 characters; contains 88 characters
 39 | ERROR   | [x] Expected 1 newline at end of file; 0 found
------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/CrosswordGenerator.class.js
---------------------------------------------------------------------------
FOUND 13 ERRORS AFFECTING 4 LINES
---------------------------------------------------------------------------
 133 | ERROR | [ ] Inline comments must end in full-stops, exclamation
     |       |     marks, or question marks
 279 | ERROR | [x] Expected 1 space before "-"; 0 found
 279 | ERROR | [x] Expected 1 space after "-"; 0 found
 279 | ERROR | [x] Expected 1 space before "+"; 0 found
 279 | ERROR | [x] Expected 1 space after "+"; 0 found
 282 | ERROR | [x] Expected 1 space before "-"; 0 found
 282 | ERROR | [x] Expected 1 space after "-"; 0 found
 282 | ERROR | [x] Expected 1 space before "+"; 0 found
 282 | ERROR | [x] Expected 1 space after "+"; 0 found
 317 | ERROR | [x] Expected 1 space before "?"; 0 found
 317 | ERROR | [x] Expected 1 space after "?"; 0 found
 317 | ERROR | [x] Expected 1 space before ":"; 0 found
 317 | ERROR | [x] Expected 1 space after ":"; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Time: 243ms; Memory: 13.25Mb

Source: http://pareview.sh/ - PAReview.sh online service

marabak’s picture

Status: Needs work » Needs review

All errors are now fixed.

arnaud.dabouis’s picture

Hi, I did a manual review.

1- file crossword_generator.module, line 34
The label is not translatable

2- file crossword_generator.module, line 92
The implemented hook is not mentioned in the comment.

3- file crossword_generator.info, line2
The description should begin with an uppercase letter.

Except for these 3 errors, everything looks great in terms of functionality and code standards.

marabak’s picture

1 & 3 fixed.
2 : crossword_generator_crossword_field_process() is a process function for crossword_field element not a hook.

arnaud.dabouis’s picture

Ok for 1 and 3. My bad for 2, you are right.

mlmoseley’s picture

Individual User Account
Yes

Duplication
Yes.

Master Branch
Yes

Licensing
Yes

3rd Party Assets
Yes

README.txt/README.md
Text is insufficiently explanatory and uses incorrect terminology. The module folder is not a 'webform directory'. The phrase 'Create a new content of type "Crossword" is incorrect english. It should be 'Create a new node of the content type "Crossword". Also the the instructions seem to dictate that the content type be named 'Crossword', when that is not necessary.

Readme does not note that you need to specify a multi value field to get multiple words in the puzzle.

Secure Code
Yes

Coding Style
Seems fine.

marabak’s picture

About the README.txt.

  • 'webform directory' was removed
  • all about content types was removed, because the module adds only a new field type and this field can be used in any content.
  • the fact that you must create a multi value field to have multiple crossing words was added too
zbombicz’s picture

Status: Needs review » Needs work

Hi marabak,

Automated Review

Minor pareview warnings:

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt

----------------------------------------------------------------------

FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES

----------------------------------------------------------------------

21 | WARNING | Line exceeds 80 characters; contains 93 characters

28 | WARNING | Line exceeds 80 characters; contains 82 characters

----------------------------------------------------------------------

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. (MIT LICENSE)
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. (+) crossword_generator.module, line 83: The theme crossword_field is not defined.The module drops warnings on all pages with crosswords (Theme hook crossword_field not found.)
  2. The css file is directly applying css on frequently used classnames, and might break the displaying of other modules content. It should be use a parent unique selector, for example: div.field-type-crossword-field .center
  3. Just a recommendation: use hook_help for including your readme file into drupal UI: https://www.drupal.org/node/161085#hook_help
  4. Just a recommendation: crossword_generator.module, line 115: There is no need to add multiple values FIELD_BEHAVIOR_DEFAULT variable here, because this is the default in hook_field_formatter_info.
  5. Just a recommendation: crossword_generator.js, line 10: define tag variable explicitly with var statement.

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.

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

marabak’s picture

Status: Closed (won't fix) » Needs review

Following zbombicz recommendations:

Coding style & Drupal API usage:
1 - no more theme for crossword_field and no more warnings
2 - more restrictive css rules : all prefixed width .field-type-crossword-field
3 - hook_help() implemented
4 - crossword_generator_field_formatter_info() cleaned
5 - crossword_generator.js cleaned

PA robot’s picture

Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxmarabak2430325git

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

marabak’s picture

Status: Needs work » Needs review

all preview errors and warnings were handled

blanca.esqueda’s picture

StatusFileSize
new20.21 KB

Hi,
I loved the idea, maybe later on as improvement a review button can be added to check the words on the crossword.

Testing your module I found out that sometimes the following message is displayed:
crossword:
Failed to find crossword.

That makes sense when the words doesn't have a matching letter. But I tested with a few words (image attached), and even after refreshing the page 10times didn't get the crossword.

marabak’s picture

Status: Needs review » Needs work

ok i'will check it - so i change the status to "needs work"

your suggestion is good - i think i will change the crossword to a drupal form with a validation process and a submit button that will check if the crossword is properly filled

marabak’s picture

The issue is due to the third party code that i'm using..
A workaround is to change the order of the words. I'm still on it :)

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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