[D7] Bootstrap Quicktabs [#2563333]

Description:
Bootstrap Quicktabs provides a "renderer" for the Quicktabs module that enables site-builders to add Bootstrap-style tabs or pills to a page. This module works with the Bootstrap theme. This is adapted from the BH Bootstrap Quicktabs under the BH Bootstrap Extras project. That project was abandoned and I was granted ownership of it. I updated fixed the remaining issues in the 1.x branch and then created a new 2.0 branch in that project with all the new features. That code is now essentially the same as this code. However, I would like to make this project the "official" module, essentially renaming the other module since it no longer contains support for the other sub-modules.

Since Bootstrap 2 only supports some of the style options and Bootstrap 3 supports new options but drops options from Bootstrap 3, there is an extras folder that contains css files with styles that will add support for the "unsupported" options.

Project:
https://www.drupal.org/sandbox/shelane/2547861

SCM:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/shelane/2547861.git

Manual Reviews:
https://www.drupal.org/node/2561547#comment-10301919
https://www.drupal.org/node/2565825#comment-10312307
https://www.drupal.org/node/2566107#comment-10324763

Comment	File	Size	Author
#19	coder-results.txt	2.55 KB	klausi
#9	Screen Shot 2015-09-16 at 12.30.49 PM.jpg	16.44 KB	kevincrafts
#9	Screen Shot 2015-09-16 at 12.30.28 PM.jpg	16.27 KB	kevincrafts

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

4 September 2015 at 15:53

shelane created an issue. See original summary.

Comment #2

PA robot CreditAttribution: PA robot commented 4 September 2015 at 16:23

Status:

Needs review

» Needs work

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

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.

Comment #3

shelane

English

CreditAttribution: shelane commented 4 September 2015 at 18:14

Issue summary:

View changes

Comment #4

shelane

English

CreditAttribution: shelane commented 18 September 2015 at 21:54

Status:

Needs work

» Needs review

I correct the issues as much as I understood what to do.

The remaining comments are for things like: There must be no blank line following an inline comment.
However, the ones that it is referring to are comments at the end of the function.

This error: 122 | ERROR | [ ] Protected method name "BootstrapQuickTabs::build_tablinks" is not in lowerCamel format

Which is true because it's a class name and then using the standard of an api from the Quicktabs module. So according to the standards, class names should begin with upper case.

This error: 144 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, or question marks

is for an end of function comment. Should these not be done.

This error: 31 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, or question marks

is for this comment section within a function:
// We're only testing here for the VERSION of jquery since
// -- this module lists jquery_update as a requirement, so we don't need
// module_exists() etc, and
// -- the variable we're checking just can't exist without the module in
// any case

I'm not sure how that should be changed.

I don't believe any of these should be application blockers.

Comment #5

shelane

English

CreditAttribution: shelane commented 7 September 2015 at 16:16

Issue summary:

View changes

Comment #6

shelane

English

CreditAttribution: shelane commented 10 September 2015 at 02:08

Issue summary:

View changes

Comment #7

shelane

English

CreditAttribution: shelane commented 13 September 2015 at 15:35

Issue summary:

View changes

Comment #8

shelane

English

CreditAttribution: shelane commented 13 September 2015 at 15:45

Issue tags:

+PAreview: review bonus

Comment #9

kevincrafts CreditAttribution: kevincrafts commented 16 September 2015 at 18:31

File	Size
Screen Shot 2015-09-16 at 12.30.28 PM.jpg	16.27 KB
Screen Shot 2015-09-16 at 12.30.49 PM.jpg	16.44 KB

I think this module is a good idea for people using bootstrap themes. The tabs worked great. However, the pills on right, pills on left did not work for me.

Comment #10

shelane

English

CreditAttribution: shelane commented 17 September 2015 at 01:38

@kevincrafts: were you using Bootstrap 2 or Bootstrap 3? Did you add the css styles from the extras folder for your version?

Comment #11

drupalove CreditAttribution: drupalove as a volunteer commented 18 September 2015 at 09:54

Hi @shelane, If there are css styles to be added please state so in the project description.

On the project description, mention the dependencies and link to them, i.e. bootstrap theme, quicktabs, jquery_update, and which jquery version is supported; e.g. 1.7 or higher. Also, which drupal version.

It would be great if you could make the comments more descriptive and more accurate when fixing the errors reported by the automated review.

It would be great if you can add an image showing the tabs.

Thanks for your contribution.

Comment #12

shelane

English

CreditAttribution: shelane commented 18 September 2015 at 16:54

Issue summary:

View changes

Comment #13

shelane

English

CreditAttribution: shelane commented 18 September 2015 at 22:00

@drupalove

The project does state that it comes with additional css for the different bootstrap versions. I also updated the language in my application.

I linked to the various dependencies (thank you) and noted that the version of jQuery depends on the version of Bootstrap and users should see the Bootstrap documentation for their version of Bootstrap to know which version is required. The drupal version will be apparent with the release name of 7.x-x.x.

I updated my comments about the errors that remain from the automated review - the items I did not change.

I will add screen shots of the tab/pill appearances next week.

Thanks.

Comment #14

shelane

English

CreditAttribution: shelane commented 21 September 2015 at 19:18

I have uploaded screen shots for all the combinations of Tabs/Pills on Bootstrap 2 and Bootstrap 3 on my sandbox project page.

Comment #15

kylebrowning CreditAttribution: kylebrowning as a volunteer commented 21 September 2015 at 19:29

Status:

Needs review

» Needs work

Line 51 needs to use Drupal API's

Line 60 should be fixed as well.

You can use theme_item_list

Comment #16

shelane

English

CreditAttribution: shelane commented 22 September 2015 at 15:45

Status:

Needs work

» Needs review

@kylebrowning

I have updated the function to use theme_item_list.

Thanks

Comment #17

kylebrowning CreditAttribution: kylebrowning as a volunteer commented 22 September 2015 at 15:46

Status:

Needs review

» Reviewed & tested by the community

looks good!

Comment #18

shelane

English

CreditAttribution: shelane commented 20 October 2015 at 19:35

Priority:

Normal

» Critical

Comment #19

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 23 December 2015 at 21:30

Status:

Reviewed & tested by the community

» Fixed

File	Size
coder-results.txt	2.55 KB

Review of the 7.x-1.x branch (commit 5e9fe10):

Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

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.

manual review:

bootstrap_quicktabs_requirements(): you are using get_t() wrong here, it does not take any arguments. It only returns the function name that should be used to translate strings. See https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/get_t/7

But otherwise looks good to me!

Thanks for your contribution, shelane!

I updated your account so 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:

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.