Closed (fixed)
Project:
Gender field
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
14 Apr 2018 at 06:16 UTC
Updated:
24 May 2018 at 15:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
alexdmccabeComment #3
alexdmccabeComment #4
krismaye commentedComment #5
krismaye commentedThis will open the help page when the module is installed (as opposed to providing a link to click).
Comment #6
krismaye commentedComment #7
alexdmccabeOkay, cool, this is a really good start! I like that you actually included the docblocks in there! A lot of people skip that, in my experience.
I do see a couple of things that need tweaking, though:
help.pageand the path is/admin/help/{name}, so that looks like the one we want. The tool you're looking for here is Url::fromRoute().Comment #8
alexdmccabeKeep it up though! And feel free to reach out, I'm not sure I was super clear with that last comment.
After we get the actual linking hammered out, we can look at adding tests to cover the change!
Thanks for the help!
Comment #9
krismaye commentedComment #10
krismaye commentedThis is a work in progress - installing the module causes the following error:
Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("name") to generate a URL for route "help.page"
Comment #11
alexdmccabeFirst off, I'm sorry, this is a huge wall of text, but I hope it's useful to you.
Okay, so the error first: looking at the route, it says the controller/method is
HelpController::helpPage.If you open
HelpController.php(in PHPStorm, you can use the Navigate menu, and then use the Class or File menu item, and then enter HelpController), you'll findHelpController::helpPageon line 114.The docblock provides the bit we need:
So, in our case, the parameter we need to pass in is the name of the module,
gender.We can find out how to make that happen by reviewing the API docs for Url::fromRoute - when in doubt, start googling "drupal" plus the class and method names you're using and/or from the error. You've got the first method parameter in there, which is
$route_name, but you're missing the second method parameter,$route_parameters.The route parameters are found in the path on the route, which in this case is defined in
help.routing.yml. The path is/admin/help/{name}. Sections in curly braces, like{name}, are the parameter(s). So this route has one parameter that needs to be passed in to the second argument of the method, as an array where the keys are the route parameter names, and the values are what you want to replace the parameter with.It's important to note that the keys in the array must match the route parameter names, or it won't work.
The other thing I wanted to note is that you're creating a link and dropping it into the
t()function.Using the translation function is a bit tricky, so don't worry if you don't get it right away. I've got a couple of links here that are worth bookmarking, I use them as a reference a lot.
Dynamic strings with placeholders
Dynamic or static links and HTML in translatable strings
Honestly, as much as I don't want to just throw some links at you and say "go read this", it's a relatively complicated system and those pages do a really good job of explaining it. It's also worth nothing that these docs were written for Drupal 7, not 8, so some things like functions names will have changed.
I'm actually going to cheat a bit here, rather than making you read two lengthy docs (although they're worth a read), and just provide you with the answer for how that should look. I'm leaving it a bit up to you still to put all this together!
With that said, I will call out a couple of sections from those docs that are relevant:
From the first link:
This is relevant for links, too. Since URLs can change, that would create new variations to be translated.
From the second link:
This section is why I put the link markup in the code, and why you can skip using
Link::fromTextAndUrl().It might also be worth digging through core a bit, if you look at
node.module, line 114, there's an example of links to a help page, actually.Sorry for the wall of text. I hope that helps, but if it doesn't, or if you still have questions, feel free to reach out again!
Comment #12
krismaye commentedComment #13
krismaye commentedComment #14
alexdmccabeLooks good, just needs tests now. krismaye is working on that.
Comment #15
krismaye commentedComment #16
sparklingrobotsComment #17
krismaye commentedComment #18
krismaye commentedComment #19
krismaye commentedFixed a few coding standard issues from the previous patch.
Comment #20
alexdmccabeI think this look good!
My only comment is that this is a Functional test, since it extends the
BrowserTestBaseclass, not theUnitTestCaseclass. That means it should be in the Functional directory/namespace alongside the existing tests, not in the Unit directory/namespace.The docblock on
BrowserTestBaseexplains:Make that change, and I think we're good to go here!
As always, let me know if you have any questions.
Comment #21
alexdmccabeComment #22
krismaye commentedComment #23
krismaye commentedComment #24
krismaye commentedComment #25
krismaye commentedComment #26
krismaye commentedComment #28
alexdmccabeDone!
Thanks a lot, @krismaye!
Comment #29
alexdmccabe