This module adds Plupload as a source through integrations of Plupload Integration module and FileField Sources module, and thus enabling multiple file uploads across a wide range of browsers.

Since FileField Sources does not have its own widget, but rather extends the core File field widgets (file and image) and other contributed file widgets, this gives great flexibility not only in file sources, but also on the type of widget. A common use case for images is to use this with a field widget for cropping.

This is a Drupal 7 project.

Sandbox: http://drupal.org/sandbox/atlea/1414774
Git:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/atlea/1414774.git filefield_sources_plupload
cd filefield_sources_plupload
CommentFileSizeAuthor
#1 drupalcs-result-1417606.txt509 bytesmorgothz
filefield_sources_plupload.jpg32.36 KBatlea

Comments

morgothz’s picture

morgothz commented
Status: Needs review » Needs work
StatusFileSize
new drupalcs-result-1417606.txt509 bytes

Review of the 7.x-1.x branch:

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. Get a review bonus and we will come back to your application sooner.

atlea’s picture

atlea commented
Status: Needs work » Needs review

Thank you for starting the work on this! Extra whitespace in plupload.js has been removed and commited. Looking forward to a manual review!

slashrsm’s picture

slashrsm commented
Status: Needs review » Needs work

I've took a look at your module. At first I must say, that module looks really solid in well written. Congratulations!

Anyway... I noticed some things, that could be improved a bit:

  • There is no CHANGELOG.txt file and it is recommended to have one. Some info at: http://drupal.org/node/52287.
  • If some day happens that filefield_sources start caching output from hook_filefield_sources_info() you have problem with including plupload.inc file, as that hook would not be called every time. It would be nice if you could implement this include on another, more robust way.
  • I noticed some spelling errors in comments. One of them can be found in plupload.inc, line 37.
  • Since you use your theme hook only to add a wrapper around your form element, it could be changed with '#prefix' and '#suffix' attributes on the same element.

As far as I am concerned can this pass when you fix this things or explain why you think it is better to have it the current way. Anyway, it would be nice if another reviewer takes a look at this module.

atlea’s picture

atlea commented
Status: Needs work » Needs review

@slashrsm: Thank you for your review. I have implemented your suggestions.

  1. CHANGELOG.txt added
  2. It does not make sense to have a separate include file if I can not load it in the info-hook, so I dopped using a separate file and merged the include file with the .module-file. It's such a small module anyway.
  3. Typos are now fixed. I hope!
  4. Yeah, I know. I was just trying to follow the same pattern as the other filefield sources. But I agree, and have implemented your changes.

Setting this to needs review again, and hope the next reviewer agree with you that the module can pass now. :)

slashrsm’s picture

slashrsm commented

That's great. Hope someone else takes a look at your module soon.

slashrsm’s picture

slashrsm commented
Status: Needs review » Reviewed & tested by the community

It looks like noone plans to review this. As I think you fixed everything, I mark this as reviewed.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Fixed

Review of the 7.x-1.x branch:

  • Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards).
    
FILE: .../modules/pareview_temp/test_candidate/filefield_sources_plupload.module
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
   6 | ERROR | Additional blank line found at the end of doc comment
  25 | ERROR | Additional blank line found at the end of doc comment
  86 | ERROR | Additional blank line found at the end of doc comment
  98 | ERROR | Additional blank line found at the end of doc comment
 184 | ERROR | Additional blank line found at the end of doc comment
--------------------------------------------------------------------------------

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. Get a review bonus and we will come back to your application sooner.

But that are just minor issues, so ...

Thanks for your contribution, atlea! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

atlea’s picture

atlea commented

Wohoo! Thanks, klausi - and a big thanks to slashrsm!

Atle

slashrsm’s picture

slashrsm commented

Great! Congrats and welcome!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Added links to Plupload Integration and FileField Sources modules