Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#24 | drupal-1979030-24.patch | 4.47 KB | dawehner |
#22 | drupal-1979030-22.patch | 4.48 KB | dawehner |
#22 | interdiff.txt | 2.47 KB | dawehner |
#14 | help-controller-2.patch | 4.69 KB | marcingy |
#12 | help-controller.patch | 4.69 KB | marcingy |
Comments
Comment #1
marcingy CreditAttribution: marcingy commentedGoing to look at this today
Comment #2
marcingy CreditAttribution: marcingy commentedPasses help tests locally so lets see what the bot thinks
Comment #4
marcingy CreditAttribution: marcingy commented#2: convert-help-page.patch queued for re-testing.
Comment #5
tim.plunkettIf you're not going to inject anything, you don't need ControllerInterface
Ideally $output would be a render array, and you would do $output[] = array('#theme' => 'item_list', ...
Comment #6
dawehnerThat's another place where the module handler could be used.
Comment #7
marcingy CreditAttribution: marcingy commentedThe issue with the render array approach is that hook_help returns strings which opens up a much wider issue and introduces so
many chance for bikesheding. Lets deal with any changes in this area in a follow up?
The module manager is now being injected
Comment #8
marcingy CreditAttribution: marcingy commentedComment #9
marcingy CreditAttribution: marcingy commentedNeed to fix up docs and should also have used an interface for module handler.
Comment #10
marcingy CreditAttribution: marcingy commentedFixes up docs hopefully.
Comment #12
marcingy CreditAttribution: marcingy commentedOK I forgot to pull.
Comment #14
marcingy CreditAttribution: marcingy commentedMissed some changes in re-roll.
Comment #15
ParisLiakos CreditAttribution: ParisLiakos commentedunneeded change
this should be turned to a render array
this should be removed
Comment #16
marcingy CreditAttribution: marcingy commentedSee #7 re render array changing it has much bigger implication as hook_help returns strings. Other parts I'll look to fix up.
Comment #17
dawehnerWell the rest of the function can still use ['#markup'] for example.
Comment #18
marcingy CreditAttribution: marcingy commentedIt can't as it is concating a string and array potentially :)
Comment #19
dawehnerManually testing this patch works for me.
Comment #20
tim.plunkettThis hunk should be reverted
This should return a render array. Let's take the extra steps here to do that, please. (But if we don't, this should be lowercase "string")
$output[$name]['#markup'] = render($temp);
or similar should suffice.And add a @todo to have hook_help always return a render array.
Yeah, let's switch this to '#theme' => 'links', here.
Comment #21
marcingy CreditAttribution: marcingy commentedSo in my eyes a half baked changed to a render array is pointless because well it is isn't an array that can truly be manipulated and of these render array changes should be a follow up.
Comment #22
dawehnerI disagree that half baked changes are bad. Small improvements are also worth to be made.
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedagreed, thanks for pushing this!
i guess its array now:)
needs removal
Comment #24
dawehnerThanks for your review!
Fixed that.
Comment #25
ParisLiakos CreditAttribution: ParisLiakos commentedlooks good, and works as expected
Comment #26
alexpottCommitted 5a31f62 and pushed to 8.x. Thanks!