[D7] Views Responsive Grid [#1879066]

Drupal 7

http://drupal.org/sandbox/iwhitcomb/1879012

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/iwhitcomb/1879012.git views_responsive_grid

Views Responsive Grid provides a views plugin for displaying content in a responsive(mobile friendly) grid layout. Rather than trying to force the standard Views grid display to work for mobile this provides the same functionality, but in DIVs instead of tables. Provided is also the ability to specify a horizontal or vertical grid layout which will properly stack the content on a mobile display.

Requirements:
Views

PAReview Bonus:
http://drupal.org/node/1907524#comment-7053140
http://drupal.org/node/1879592#comment-7053272
http://drupal.org/node/1702136#comment-7053492

Comment	File	Size	Author
#1	drupalcs-result.txt	1.5 KB	robinvdvleuten

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

robinvdvleuten CreditAttribution: robinvdvleuten commented 3 January 2013 at 16:38

Status:

Needs review

» Needs work

File	Size
drupalcs-result.txt	1.5 KB

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

Remove "version" from the ./views_responsive_grid.info file, it will be added by drupal.org packaging automatically.
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

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.

Manual review:
I see some IE7 hacks in the stylesheet. Can you tell which browsers are supported by the module? Maybe you can put this in the description of your module's sandbox. If the stylesheet is only used for an IE7 hack, you should modify the drupal_add_css call at line 18 in views_responsive_grid.module so it is only loaded when the browser is IE7.

Cheers,

Robin

Comment #2

iwhitcomb CreditAttribution: iwhitcomb commented 3 January 2013 at 19:19

Status:

Needs work

» Needs review

Ok, we got rid of the inline and module CSS in favor of using user-defined row/column classes which in turn would be handled at the theme layer.

Fixed some of the PAReview errors. It keeps croaking on the view plugin class, which follows the standard set by views, but isn't supported by PAReview.

Comment #3

robinvdvleuten CreditAttribution: robinvdvleuten commented 4 January 2013 at 10:47

It is not only about the name of the class. There are also some notes about the docblock comments of the methods and their scope (public/private). I assume that the methods are public or otherwise protected.

Comment #4

iwhitcomb CreditAttribution: iwhitcomb commented 5 January 2013 at 02:17

Yeah, I know, I added the two comments, but that's pretty minor. See views/plugins/views_plugin_style_grid.inc it's exactly the same.

Comment #5

monymirza

Urdu

Islamabad

CreditAttribution: monymirza commented 5 January 2013 at 06:53

Status:

Needs review

» Needs work

Hi,

There still some errors/warnings pending that need to fix. Please use "Coder" module before pushing your code to Git.

you can find issues here:
http://ventral.org/pareview/httpgitdrupalorgsandboxiwhitcomb1879012git

Comment #6

iwhitcomb CreditAttribution: iwhitcomb commented 5 January 2013 at 07:44

Status:

Needs work

» Needs review

Yes, see above. The errors being returned by PAReview are because Views module chose not to format their plugins according to the Drupal coding standards.

Comment #7

srutheesh CreditAttribution: srutheesh commented 9 January 2013 at 07:13

Hi,

In the file 'views_responsive_grid_plugin_style_responsive_grid.inc' please fix the following minor error.

At line no -14 function comment short description must end with a full stop.

Comment #8

iwhitcomb CreditAttribution: iwhitcomb commented 10 January 2013 at 17:33

Fixed, thanks.

Comment #9

iwhitcomb CreditAttribution: iwhitcomb commented 7 February 2013 at 14:55

Priority:

Normal

» Critical

4 weeks without review -- changing to critical status.

Comment #10

iwhitcomb CreditAttribution: iwhitcomb commented 11 February 2013 at 16:56

Issue tags:

+PAreview: review bonus

Took care of PAReview:
http://drupal.org/node/1907524#comment-7053140
http://drupal.org/node/1879592#comment-7053272
http://drupal.org/node/1702136#comment-7053492

Comment #11

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 11 February 2013 at 22:43

Assigned:	Unassigned	» cweagans
Priority:	Critical	» Normal
Status:	Needs review	» Reviewed & tested by the community
Issue tags:	-PAreview: review bonus

manual review:

views_responsive_grid_views_api(): doc block is wrong?

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Comment #12

cweagans

He/Him

English

Boise, ID, USA

CreditAttribution: cweagans commented 12 February 2013 at 00:40

Status:

Reviewed & tested by the community

» Fixed

I took a look at this too, and it looks good to me. Many thanks for your contribution and reviews!

I updated your account to let you 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:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get 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.