Provide an example contextual links implementation (using MENU_CONTEXT_PAGE and MENU_CONTEXT_INLINE)

Yay!

The above patch looks like a really great start.

But I think to really demonstrate how to use contextual links in Drupal 7, we should do more than that. Attaching contextual links to a node is the easiest example, but there are other tasks that many modules will want to do that are a bit more complicated (e.g., attaching contextual links to a block). It would be great if the Examples module could show people how that is done also.

So here's an attempt to expand the patch to cover those. I worked on this mostly during the code sprint last Friday at DrupalCon but finished it now. I built on what the previous patch did (just corrected a couple small mistakes in the code comments) but then added a separate "Contextual links example" module to show the full functionality.

Since attaching contextual links to nodes is an important example and also a simple case that only involves the menu system, for now I've duplicated that example in both places (i.e., left it in the "Menu example" module as per the previous patch, but also added it to the "Contextual links example" module so that module's example is complete). Not sure if that duplication makes sense, but I think it does.

Hm, an invalid patch format due to non-Unix line endings... that is one I've never seen before. Especially since I make my patches on Linux :)

But I think I created the new module's .info file by copying one from somewhere else, and I guess that's where the non-Unix line endings snuck in. This should hopefully fix it.

Just some ideas:

For the sake of usability, could some additinal examples be put as snippets in a readme or advanced-readme file or as advanced-help content out of the module code? too much code and comments can be distracting, and describing all the possilibities of each option for a single API as an example can mess the code, plus more code = more tests, harder to maintain, making the idea of a 'module script' difficult to follow.

My bet is keep example's code simple, and provide additional support out of that, in this case we are talking about documentation, so advanced-help could be an improvement, but an additional readme file with a few 'replace' snippets could be enough. Some of the options would turn a final example more descriptive code than usable code. Examples should (IMO) provide a kinder-garden for developers, touching/changing/replacing things IS a must to understand how the API or core or functionality works, extensively describing each is also good, implementing them all is out of the scope. I recall: my opinion.

Regarding the contextual menu entries, additional information is needed, I'd say: when is recommended to use it, when it is not? will all themes handle it? what is expected in a theme to render that item in the right place?

Note: I'm +1 to include them, not sure if we should create a new submodule for menu entries however, so I'm accepting other's suggestions here.

Please, consider that this module already has good documentation, tests (do they really show up on the test list page?), and a very specific use case. I can't see the point about patching also menu_example module to make this working.

I just noticed that my english is getting worse the more I write, brbr

I think attaching contextual links to nodes and blocks are essential examples that a lot of developers will use (in some ways, I'd expect the block example to be the most common one, actually).

The third example I gave is a bit less common, although with everybody moving to use entities for all their modules in D7, I don't think it's that uncommon. (If you want to define or do anything interesting with an entity that has a UI, you are likely going to want contextual links.) I don't see how moving it to a README would be beneficial - it really seems better to have a working example piece of code that people can play with.

One thing that might help (I'm not sure) would be to move it out to a separate module? The main contextual links example module could then just be nodes and blocks (the most common use cases) which would keep the code a whole lot simpler. The more comprehensive example would be in its own module.

Please, consider that this module already has good documentation, tests (do they really show up on the test list page?), and a very specific use case.

The test file is listed in the .info file and the tests show up in the UI (and pass) for me locally. However, I see from http://qa.drupal.org/pifr/test/132809 that the testbot did not appear to pick up these new tests and run them. I'm not sure why.

I can't see the point about patching also menu_example module to make this working.

I assume the original rationale is that 'context' is one of the keys in hook_menu() and the current menu_example module seems to go through (somewhat) comprehensively how to use all the major options. So that's why I kept it there when I updated the patch.

It's probably equally true that no one looking to understand contextual links would ever think of looking there for it, so maybe the duplication doesn't make sense after all.

One weird strangeness of the testbots currently is that new submodules or new .test files included in a patch do not get run by the bot. If I remember right there's an issue open for this. So that's why the tests here aren't being run.

I'd go for a single really simple module and provide additional 'cases' by documentation. About the tests in the patch, is this just me, or the getInfo() method in the test class is missing?

About the 'entities' comment that you mention for the contextual menu, considering the developer knowledge required to get an entity working perhaps this example is not required at all. Not sure..

So, if rfay agreeds in the format (custom module, included cases and so) then I'll review it deeply. Thanks for submitting this, David_Rothstein.

I just realized that if we split it up it may be possible to make the block example itself simpler, which is a win. I'll try to look into that soon - ideally it would be nice to get something committed here as soon as possible (even if not the full thing), since we're trying to build out http://drupal.org/ui-standards and link to Example module code from there.

About the 'entities' comment that you mention for the contextual menu, considering the developer knowledge required to get an entity working perhaps this example is not required at all.

Entities are only complex if you are trying to do complex things with them. Getting a basic entity working is pretty simple and in my opinion requires much less developer knowledge than something like a module-defined node type (which the Examples module already has code for).

I see #893842: Entity example module, although I think that particular example may already be too detailed to want to add contextual links and theme stuff to it also. I think there is room to get this code example in somewhere, though, maybe via a module that focuses on how you pass an object through the theme system (render_example.module?). Attaching contextual links is an important part of the rendering system. I'll think about it a little more. It can be moved to a separate issue if necessary.

About the tests in the patch, is this just me, or the getInfo() method in the test class is missing?

Nope, it's there in the patch.

I am in agreement with it being a separate module, as long as @David doesn't think that will distract from what people "ought to do" in the regular course of things. In other words, if we're specifically showing off contextual links, a separate module is best. If, on the other hand, *everybody* should know and use contextual links all the time, then perhaps they should be sprinkled throughout.

@Randy, That is exactly the same use case as we do have in the verticaltabs_example that has not been included in other modules and is using its own space (@David_Rothstein, remember this for that section in the UI doc). The problem here is the small size of the line splitting 'documentation' and 'example', and sometimes these modules mix both.

@David_Rothstein, committing is the easiest part, so go ahead if you want to make changes or comment if the module is going to be as in #7 to start reviewing the code.

BTW, I apologize to all that I'm still MIA from Examples. Getting the testbots stabilized after the Git migration is still my overall priority, and because that affects so many people I'm just bulldogging it with my spare time. Thanks everybody for keeping the good work going forward.

Randy, your work is much appreciated, regardless the issue queue.

In other words, if we're specifically showing off contextual links, a separate module is best. If, on the other hand, *everybody* should know and use contextual links all the time, then perhaps they should be sprinkled throughout.

Yeah, that is a good way to look at it. And I think that means putting everything in a completely separate module is best. Although people doing work with nodes or blocks might want to know about contextual links also, it's not the case that you should always attach contextual links every time you're dealing with a node or a block. So we can't, for example, easily put the block contextual links example in block_example.module -- not without adding a lot more code to that module to make it a realistic case where you'd actually need to add some contextual links.

Similarly, I thought above that I could simplify the block example I had in the previous patch, and while that's still true codewise, I no longer think it's a good idea. It's better to have an example that's more realistic, where you'd actually have a good reason to add a new contextual link to the block.

So based on that, here's an updated version of the patch. The main thing I changed is that to address one of @ilo's points in #8, I added more code comments about when it is and isn't recommended to use contextual links in different situations. Put together, I think that makes the whole module now tell a more realistic story that will make sense to someone who wants to play around with contextual links, so for now I haven't removed any of the examples themselves.

This patch is ready for a review if anyone has time to look into it - thanks!

From a quick reading, I like the idea of three use cases, but I think it can be simplified a little bit. I'll review it and come back with comments.

Thanks for all the work you have done on this, David_Rothstein, now that I look at it, I've found the module very useful.

For reference (and on the topic of documentation), I just came across this page which tries to provide some documentation of how to implement contextual links in code: http://drupal.org/node/1089922

Although it kind of seems like it's located in an odd place in the handbook...

Starting to look at the contextual code - how did this get into core?!

Among other things - why does it need to touch hook_menu, and why can't I use a page callback as a contextual link? This is making me very sad.

Basically it seems that - http://api.drupal.org/api/drupal/includes--menu.inc/function/menu_contex... should NOT be used for contextual links for blocks. For blocks, I want to simply use the path I specify to build the link.

Fantastic. Sorry I've been dragging my feet so long!

I think this is great. It helps so much with this issue.

I did some tiny cleanup, added the link you mentioned, @David_Rothstein, and committed:
D7: a115cb9
D8: e5abb80

Great! Thank you, @rfay.

This patch introduced D7 code into the D8 branch. Fixed in commit c0f027551980.

@eojthebrave, @David_Rothstein would you mind taking a look at #1310536: Contextual Links Example completely broken (Edit object doesn't work in block) - it seems that 8.x version doesn't work any more.