Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#16 help_controller-1979028-16.patch5.54 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,401 pass(es).
[ View ]
#16 interdiff.txt607 bytesplopesc
#14 help_controller-1979028-14.patch5.54 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]
#14 interdiff.txt1.76 KBplopesc
#12 help_controller-1979028-12.patch3.79 KBplopesc
FAILED: [[SimpleTest]]: [MySQL] 55,427 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#12 interdiff.txt1.66 KBplopesc
#10 help_controller-1979028-10.patch3.71 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,424 pass(es).
[ View ]
#10 interdiff.txt1.48 KBplopesc
#8 help_controller-1979028-8.patch3.49 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,403 pass(es).
[ View ]
#8 interdiff.txt1.67 KBplopesc
#5 help_controller-1979028-5.patch3.08 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,448 pass(es).
[ View ]
#5 interdiff.txt850 bytesplopesc
#3 help_controller-1979028-3.patch3.17 KBfrob
PASSED: [[SimpleTest]]: [MySQL] 55,866 pass(es).
[ View ]
#3 interdiff.txt579 bytesfrob
#1 help_controller-1979028-1.patch2.64 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 55,346 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.64 KB
PASSED: [[SimpleTest]]: [MySQL] 55,346 pass(es).
[ View ]

Hello

Attaching patch that converts help_main() to a Controller.

Regards

Assigned:Unassigned» frob

StatusFileSize
new579 bytes
new3.17 KB
PASSED: [[SimpleTest]]: [MySQL] 55,866 pass(es).
[ View ]

The patch applied cleanly. Patch 1 didn't change the original hook_menu to use 'route_name'.

Rerolled patch should fix that.

Assigned:frob» Unassigned

StatusFileSize
new850 bytes
new3.08 KB
PASSED: [[SimpleTest]]: [MySQL] 55,448 pass(es).
[ View ]

Nice catch @frob, I forgot to include help.module changes in my previous patch.

Re-rolling patch indenting doc and removing unused __construct() method in HelpController class.

Regards

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,70 @@
+    foreach (module_implements('help') as $module) {
+      if (module_invoke($module, 'help', "admin/help#$module", $empty_arg)) {

Both module_implements and module_invoke could be replaced by the module handler: 'module_handler' which is in the container.

Assigned:Unassigned» plopesc

Yeah! go for it!

StatusFileSize
new1.67 KB
new3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 55,403 pass(es).
[ View ]

Re-rolling patch using the module handler.

Thank you.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+  public function __construct(ModuleHandler $module_handler) {

This should use the interface and could get some documentation on the object.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+   * @return

Should be @return string

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+  }

You should have an empty line before the last } of the class.

StatusFileSize
new1.48 KB
new3.71 KB
PASSED: [[SimpleTest]]: [MySQL] 55,424 pass(es).
[ View ]

Re-rolling patch according to last advices.

@dawehner Thanks for your efforts.

Beside from these two small things, this is ready to fly!

+++ b/core/modules/help/help.moduleundefined
@@ -12,8 +12,7 @@ function help_menu() {
     'file' => 'help.admin.inc',

No need for file anymore. sorry I should have seen this before.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+   * @var \Drupal\Core\Extension\ModuleHandler
...
+   * @param \Drupal\Core\Extension\ModuleHandler $module_handler
...
+  public function __construct(ModuleHandler $module_handler) {
+    $this->moduleHandler = $module_handler;

:( Should be ModuleHandlerInterface

StatusFileSize
new1.66 KB
new3.79 KB
FAILED: [[SimpleTest]]: [MySQL] 55,427 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

No worries, here is the definitive patch, I hope :)

Regards.

Oh, is there a reason why you don't remove the previous code?

StatusFileSize
new1.76 KB
new5.54 KB
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]

Oh sorry, it was a regression generating patches.

Re-rolling...

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+  private function helpLinksAsList() {

Urgs. Drupal just uses proctected

StatusFileSize
new607 bytes
new5.54 KB
PASSED: [[SimpleTest]]: [MySQL] 55,401 pass(es).
[ View ]

Re-rolling changing function definition.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+    $output = '<div class="clearfix"><div class="help-items"><ul>';
+    $i = 0;
+    foreach ($modules as $module => $name) {
+      $output .= '<li>' . l($name, 'admin/help/' . $module) . '</li>';
+      if (($i + 1) % $break == 0 && ($i + 1) != $count) {
+        $output .= '</ul></div><div class="help-items' . ($i + 1 == $break * 3 ? ' help-items-last' : '') . '"><ul>';
+      }
+      $i++;
+    }
+    $output .= '</ul></div></div>';

We should certainly open a follow up to convert this into a themable function.

Yeah, a follow up issue for clean the way the output is generated could be a good option.

Then, we could mark as RTBC this issue if you consider it and open a follow up taking as base this code once it will be committed.

What do you think?

Regards.

Status:Needs review» Reviewed & tested by the community

Sounds like a great plan.

Status:Reviewed & tested by the community» Fixed

Committed 51dc76b and pushed to 8.x. Thanks!

Follow-up issue created: #1986164: Improve the way main help page is rendered

Regards.

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