Adding contextual links via Views fields does not work

Did you updated from an older Drupal core version to beta 12? In case yes,
please try out the head2head module and there especially the beta2beta module.

Note: We don't have an update path yet in core: https://www.drupal.org/node/2341575

Until we get confirmation of whether this was a new install

I see exactly the notices mentioned in the summary, testing a fresh Drupal 8.01 (new install). My goal was to add a contextual edit link to a view of fields. Steps to reproduce:

- Install Drupal 8.01
- Create some articles
- Create a views block of articles, Display: Fields, and add some article fields, e.g. Title and Body.
- Add an edit link field to the display, and hide it.
- Add the field "Global: Contextual Links", and incluce the edit link.
- Save the view.
- Configure the block to show up somewhere.

As a result (besides the above mentioned notices), the contextual links field shows up, but it doesn't show the edit link. (Tested with Bartik.) That's what I see in the HTML:

<div class="views-field views-field-contextual-links">
  <span class="field-content"></span>
</div>

@Olafski
If you want to make the life of people easy, please provide an export of the view.

Sorry, that I missed that. Please see the attached YAML file.

I can reproduce this issue when adding the contextual links field. The notices are due to parent::buildOptionsForm not being called.

However, solving that only gets rid of the notices but the contextual links will still be empty.

During rendering, $rendered_field = $this->view->style_plugin->getField($values->index, $field); is called which looks in the rendered_fields, which is still empty during render, so this will always skip all fields.

$rendered_field = $this->view->style_plugin->getField($values->index, $field);
      if (empty($rendered_field)) {
        continue;
      }

When working around that, the contextual button is rendered but will still not work (and when used inside a table layout be unclickable due to positioning).

It's a mess. Attaching a patch to get rid of the notices mostly to point to where the trouble comes from, but this needs much more work and the ContextualLinks handler needs test coverage.

Back to contextual module, where the code lives.

This lives in the contextual module, yes, but all the complexity here is in Views:

When working around that, the contextual button is rendered but will still not work (and when used inside a table layout be unclickable due to positioning).

It's a mess. Attaching a patch to get rid of the notices mostly to point to where the trouble comes from, but this needs much more work and the ContextualLinks handler needs test coverage.

I just thought it'd have a better chance of getting the right people to look at it if it's in the Views issue queue. The contextual module doesn't need this to function. It's an enhancement for Views. (That used to live in the Views module in D7.) But, if you disagree with that, fine.

Just be aware that the contextual module has no active maintainer. I try to take on some of it, but I won't do this issue, because it goes so deeply into Views complexity.

@Wim Leers That's what the VDC tag is about, I always thought. Views related stuff that isn't in the views or views ui module.

I'll take a look at this, at least try to make it semi-functional and provide some basic test coverage for starters.

Fair enough :)

This lives in the contextual module, yes, but all the complexity here is in Views:

Well, complexity is always relative.

First steps.

Some tests that illustrate that this thing is really broken.

The fix just makes the tests pass and makes the handler a little less broken. But no actual links are rendered yet (but the trigger button is atleast now). The information passed to _contextual_links_to_id seems to be out of date. That function seems to want some sort of structured data, but I can't tell what it wants (haven't looked really hard). I just saw that the data it gets passed for the View contextual link is structured differently then what gets passed by the views handler.

So that data

      $contextual_links = array(
        'contextual' => array(
          '',
          array(),
          'route_parameters' => array(
            'contextual-views-field-links' => UrlHelper::encodePath(Json::encode($links)),
          )
        )
      );

needs to be restructured to make this work (added the route_parameters key to make the tests pass), but that complexity lives in the contextual module and I'm not really familiar with that :)

Putting it on needs review to run the testbot, but this obviously still needs work to get this working.

Looked at the information passed to _contextual_links_to_id a bit more closely but can't really find a format that actually works for this field.

After applying this patch, the content of the data-contextual-id is the following:

data-contextual-id="contextual:contextual-views-field-links=%257B%2522edit_node%2522%253A%257B%2522href%2522%253A%2522%255C/node%255C/1%255C/edit%2522%252C%2522title%2522%253A%2522edit%2522%252C%2522query%2522%253A%257B%2522destination%2522%253A%2522%255C/frontpage%2522%257D%257D%252C%2522translation_link%2522%253A%257B%2522href%2522%253A%2522%255C/node%255C/1%255C/translations%2522%252C%2522title%2522%253A%2522Translate%2522%252C%2522query%2522%253A%257B%2522destination%2522%253A%2522%255C/frontpage%2522%257D%257D%252C%2522delete_node%2522%253A%257B%2522href%2522%253A%2522%255C/node%255C/1%255C/delete%2522%252C%2522title%2522%253A%2522delete%2522%252C%2522query%2522%253A%257B%2522destination%2522%253A%2522%255C/frontpage%2522%257D%257D%257D:langcode=en"

If i bring my mouse above the item, then the contextual link icon appears, however if i click on it, then the menu does not appear.
The menu list is not in the div (<ul class="contextual-links">...</ul>).
Line 165 in contextual.js:
var html = storage.getItem('Drupal.contextual.' + contextualID);
the content of the html variable is empty, the key exists, but its empty.

Patch doesn't apply to Drupal 8.7.2

The last patch was updated.
The tests was actualized.

Was uploaded fix for the last patch

@aleevas thanks for looking at this, but this thing is so broken it's never getting fixed. This is still based on how contextual links worked in D7 and that has zero connection to how it works in D8.

Just use the Dropbutton field if you need this kind of functionality. And maybe add some styling to make it look like contextual link or something.

#30 fails to apply on 9.06 for those interested in fixing this on D9

I took a stab at this and pushed to issue fork. Did not test it though.

OK i debugged and fixed everything, we now have a field and area handler that worksforme 🌠 in MR 99.

I do not currently have bandwidth to work on test coverage, but i reviewed patch #30 and it looks like it is an excellent starting point.

(Attaching patch for my deployment workflow, please ignore.)

Now going to fix #2983655: Contextual links are not displayed correctly.

(Patch only for deployment.)

Now i'm happy with it code-wise. (Patch only for my deployment, work happens in MR.)

Huh, i'm too sleep deprived and hacked in the wrong issue. Reverting the last commit.

@geek-merlin

How can I test it?

I added field to my view "link to edit node"
I added field "contextual link"and choose my field

But nothing. I don't see contextual link near my views row

I have applied the patch from #41 and the contextual links are appearing for me. Marking RTBC.

Thanks everyone for reviving this and coming up with a D9-compatible solution.

One thing that's missing here is test coverage. An automated test was added way back in #15 for 8.0.x, so that might be a starting point, although I don't recommend using such old config exports etc. and obviously the test API has changed drastically since then. (It'd be better to recreate the fixtures if we use a similar approach for adding tests this time around.)

Oh, also, let's please write an issue summary. :) Thanks!

I think this is fixable, or at least automatically testable, but I have no idea how to reproduce it. Can someone please update the issue summary with clear steps to reproduce the problem? Postponing until then.

Reproduction steps:

• Fresh D9.2.1 install. Dutch language in this case. Don't think that is relevant.
• Create some content.
• Create a view of that content type.
• Create a view page
• Display fields
• The field Titel shows up by default.
• Add the field Link to edit content and hide it.
• Add the general Contextual links field and select to use the Link to edit the content.
• Save the view and visit it.
• You will not get the contextual link for node edit. Of course you will see the contextual link to edit the view, so do not get confused here.
• Check the logs and you will see some notices too.

I patched to https://www.drupal.org/files/issues/2021-01-31/drupal-2532200-41.patch but I had to alter this line:
From:
$rendered_field = $this->view->field[$field]->last_render;
To:
$rendered_field = $this->view->field[$field]->last_render ?? FALSE;

See uploaded file "drupal-2532200-41.png" for more details about what caused the error.

Adding steps to reproduce. Unpostponing (poning?)

Ran into this today.

Looks like this mostly needs a re-roll and tests?

Adding remaining tasks (and headings) to issue summary.

The diff in the merge request still applies (haven't tested it yet), but we need to move it to 9.5.x.

Re-rolled https://git.drupalcode.org/project/drupal/-/merge_requests/99.patch for 9.5.x.

Fixed CCF for #61.
Please review.

I've reviewed the last two patches 61 and 62 and I think I made a mistake somehow when re-rolling it. According to the latest version of changes in the merge request (for 9.2.x), we shouldn't be altering any JavaScript files.

I've merged this merge request branch into 9.5.x and here I'm uploading the diff patch, which is very similar to the merge request changes. Unless there is something wrong from the latest work in the merge request, we should be reviewing and using this one.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Previously was tagged for tests in #8 which still need to happen.

I tested 2532200-64-9.5.x.patch (#64) and it seems to be working well. Tested with Drupal 9.5.1

Added patch against #64 in 10.1 version

Hiding patch #69 as it's omitting the added tests.

Adding it to the list here: #3339883: Employees of Dotsquares are posting mass re-roll patches which are invalid and/or incomplete

#64 works with version 10.2.5 (php 8.1, mysql 8).

Confirming that #64 works great in Drupal 10.3.2!

(mea culpa: I previously complained this wasn't working, but that turned out to be a problem with my own CSS)

However, #64 does not work in Drupal 11. Specifically, the file contextual.views.inc no longer exists.

So this patch needs a bit more than just a reroll for different file structure, but reworking to follow the core changes.

OK, so on experimenting: We can remove this section from the patch. The patch then applies to Drupal 11 core, and my views with contextual links do then start to display correctly:

diff --git a/core/modules/contextual/contextual.views.inc b/core/modules/contextual/contextual.views.inc
index bac5522f15..d8059a6dd9 100644
--- a/core/modules/contextual/contextual.views.inc
+++ b/core/modules/contextual/contextual.views.inc
@@ -11,8 +11,12 @@
 function contextual_views_data_alter(&$data) {
   $data['views']['contextual_links'] = [
     'title' => t('Contextual Links'),
-    'help' => t('Display fields in a contextual links menu.'),
     'field' => [
+      'help' => t('Display fields in a contextual links menu.'),
+      'id' => 'contextual_links',
+    ],
+    'area' => [
+      'help' => t('Display links in a contextual links menu.'),
       'id' => 'contextual_links',
     ],
   ];

So:– Why is it necessary to patch contextual.views.inc to get this fixed? Can we just drop that?

If it is needed, the next step is to redo the patch, to patch core/modules/contextual/src/Hook/ContextualViewsHooks.php instead.

Re: #79:

If it is needed, the next step is to redo the patch, to patch core/modules/contextual/src/Hook/ContextualViewsHooks.php instead.

Do you mean 'If it is not needed,..' [then we can] 'patch core/modules/contextual/src/Hook/ContextualViewsHooks.php instead.'? So are you saying that if we can get rid of contextual.views.inc then we can implement the code inside that file by patching core/modules/contextual/src/Hook/ContextualViewsHooks.php?

If we can get the fix done described in the IS title, by patching/ editing contextual.views.inc then I think that is within the scope of this issue. Removing contextual.views.inc should be done in a follow-up?

BTW, Sorry I tried to make the pipeline work and run tests on the MR but it only seems to have partially succeeded. Perhaps the gitlab config file needs to be edited?

It does seem in #79 that there is a now a fix to this issue. So now we should apply the fix to the existing MR or create a fresh one if that one is wonky. Then we can test the fix in the pipeline and also get the fix reviewed.

Hi Oily

Sorry not to be clear.

The D10 and below version of Contextual has the file contextual.views.inc. The proposed fix in this issue for D10 and below includes a patch for that file. I'm unclear, reading through the history, what that change does.

The D11 version of Contextual no longer has the file contextual.views.inc. That is to work with the adjusted core API, so the code that was in contextual.views.inc is now in ContextualViewsHooks.php instead.

So I was unclear if the patch to contextual.views.inc is actually needed to fix this issue. If it is needed to fix this issue, then we patch ContextualViewsHooks.php. If changing contextual.views.inc is not actually required to fix this issue, we just remove that part of the patch.

I'd suggest starting with a fresh MR to keep it cleaner.

I'll get a patch up in a mo.

Here is the patch that omits any changes equivalent to the changes previously proposed for contextual.views.inc

And here is the patch that incorporates the changes previously proposed for contextual.views.inc into ContextualViewsHooks.php.

My hunch is that, although I cannot see why contextual.views.inc needed changing, let's assume it was for a good reason. So we probably want this patch rather than the one in the comment above.

I took a look at the MR and the changes added in #83. I created a new branch, and rebased the changes onto the latest version of the 11.x branch. These changes drop the .gitlab-ci file and commits for the js changes which aren't needed. I then converted the changes from #83, which handle the update object based hooks, and added that to the PR. I tried to resolve some of the coding standard issues that were reported in the ci job as well.

I wasn't able to easily resolve the issue with the missing return type for area\ContextualLinks::buildOptionsForm. I think the {@inheritdoc} is the correct answer here, but it appears that the definition for the method on the higher level plugin base classes is missing a return type. On a quick review, it doesn't look like the return is used at all, but I don't know if making that fix is in scope for this issue or will have any larger effects.

I think the last major thing to do it here writing tests.

Rerolled the patch from #83 to work with the latest core

Sorry, that patch got corrupted when I downloaded it ready to add to this issue. Attaching a fixed version.

I fixed the CI erorrs, added a test, and am hiding the patch files so that reviewers use the MR.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.