[D7] Ajax pages

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.

Hello,

Please check http://ventral.org/pareview/httpgitdrupalorgsandboxjamix1988012git-7x-1x

Regards

Hi, jamix.
No errors found, just don't understand some thing.

From README.txt:

* In your theme's html.tpl.php file, wrap the output of the page_top, page and
  page_bottom regions inside a div with a unique id.
  ...
* Go to the settings page at Administration -> Configuration -> User interface 
  -> Ajax pages and define the Page wrapper selector.

Why force the user to perform extra steps? I would rather implement this functionality with a hook_preprocess_html() in the module. The same goes for "Adding a persistent region".

How exactly would you implement wrapping the output of the page_top, page and page_bottom regions via hook_preprocess_html()?

I hurried. It will be easier with using hook_page_build(), like:

function  ajax_pages_page_build(&$page) {
  $page['page_top']['#prefix'] = '<div id="page-wrapper">';
  $page['page_bottom']['#suffix'] = '</div>';
}

And, more importantly, if we were to do such things for the end user, how would we know a). How many persistent regions they want...

Yes, perhaps you're right here, Artem.

How exactly would you implement wrapping the output of the page_top, page and page_bottom regions via hook_preprocess_html()?

I hurried. It will be easier with using hook_page_build(), like:

function  ajax_pages_page_build(&$page) {
  $page['page_top']['#prefix'] = '<div id="page-wrapper">';
  $page['page_bottom']['#suffix'] = '</div>';
}

And, more importantly, if we were to do such things for the end user, how would we know a). How many persistent regions they want...

Yes, perhaps you're right here, Artem.

Remove repost, please. )

Using #prefix and #suffix on $page['page_top'] and $page['page_bottom'] looks a bit hacky to me.

Why? Using Render API is a hacky for you?))

... but in some whacky situations where the user alters their html.tpl.php, that may lead to unforeseen and hard to trace bugs.

In this case, and your code will not work:
line 232, ajax_pages.module

$content = $html_variables['page_top'] . $html_variables['page'] . $html_variables['page_bottom'];

Hello jamix!

I reviewed your module:

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
54 | WARNING | Line exceeds 80 characters; contains 83 characters
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/ajax_pages.module
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
130 | ERROR | If the line declaring an array spans longer than 80 characters,
| | each element should be broken into its own line
--------------------------------------------------------------------------------

1. There is no critical errors in lines above, but it's better to fix all of them.

BTW: I'm going to use your module in my client's project. So it would be great to add a feature to apply your module just for several pages in addition for suppression Ajax pages. Like it does for blocks for example.

Thank you for your module!

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

and do not use t() in hook_menu().

Hello Klaus!

You are right about t() in hook_menu(). Шэму corrected my previous post.
I've reviewed the code manually, installed the module on my project.
I suppose the code is ok.

Thanks for your contribution, jamix! 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.

This is a solid project and i'm not going to set it back to needs work after a month at RTBC, but please do remove that t() in hook_menu().

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 reviewers!

fixed in #16.

----
Top Shelf Modules - Crafted, Curated, Contributed.