Contextual links inaccessible for attachment display, node style listing

I'm not seeing them either (and miss them!) -- no javascript errors reported. views-hide in the wrapper is always set to display none; I don't think the hover script is ever executed.

Could it be that this depense highly on the active theme?

We use core contextual links now, so they should provide a sane default.

I have the same problem and I can't see them even in Bartik theme

For me it's solved, there were no links because the view didn't show any row. But I really think the links should also be shown even if there are no results.

It might be possible to fix this via some padding or margins in the CSS... although that would affect the layout of the page so not so good either.

I have a patch at #876772: Better contextual link integration for blocks and pages which doesn't fix this problem directly, but does make it much less of a concern. In the case of blocks and pages, the patch moves the contextual links dropdown to appear in a more appropriate place (attached to the block or page title, not to the view itself). So with that patch, the problem here goes away in most practical situations.

However, it's still not a complete fix because even with that patch, if you choose to override the default behavior and go back to displaying the links right on the view itself, the problem described above would reoccur.

This patch was commited already so is it fixed now?

It would be cool to hear some feedback from you.

Since Views itself only attaches contextual links to pages and blocks now, I don't think this bug actually surfaces directly anymore.

But the API does still provide the option to attach them to the view itself, and the code to support that is in template_preprocess_views_view(). And maybe there are types of view displays where it would make sense to display them there.

So I think this should remain open. Perhaps it could be downgraded to "minor", though.

This patch adds contextual links back to all displays which doesn't provide some own special behaviour.

Okay commited this patch because we need a second beta asap.

@david_rothstein
It would be cool if you could review the patch.
Mh better review the code in git. Fixed some comments there.

Hi @dereine, could you explain more what the purpose of this patch is?

It looks to me like it makes it impossible for anyone to have a view display that doesn't show contextual links (well, at least for displays that use the standard views theming). I wouldn't think that contextual links should always be shown everywhere - e.g., should attachment displays get contextual links when the main display they are attached to already has them?

At first glance, it seems to me like the current behavior (where each view display decides for itself if it wants to opt-in to contextual links) makes more sense.

Anyway, assuming that's the behavior you do actually want, then it looks to me like most of the patch (the parts with $admin_links) isn't necessary. I think you could accomplish it with this part alone:

@@ -682,6 +682,10 @@ function views_add_contextual_links(&$render_element, $location, $view, $display
     // contextual links that are intended to be displayed in the requested
     // location.
     $plugin = views_fetch_plugin_data('display', $view->display[$display_id]->display_plugin);
+    // If contextual links locationos is empty provide a sane default.
+    if (empty($plugin['contextual links locations'])) {
+      $plugin['contextual links locations'] = array('view');
+    }

The Contextual Links module puts the links in $title_suffix, which is already used in views-view.tpl.php, so it doesn't look to me like the $admin_links variable adds anything.

Well there should be some default output for the contextual links. There are many display plugins, for example calendar which really should have one.

I'm not sure whether it's good to define it to be disabled by default.

Well there should be some default output for the contextual links. There are many display plugins, for example calendar which really should have one.

I'm not sure whether it's good to define it to be disabled by default.

OK, how about the attached patch then? The key change I made was this:

-    // If contextual links locations are empty provide a sane default.
-    if (empty($plugin['contextual links locations'])) {
-      $plugin['contextual links locations'] = array('view');
-    }
+    // If contextual links locations are not set, provide a sane default. (To
+    // avoid displaying any contextual links at all, a display plugin can still
+    // set 'contextual links locations' to, e.g., an empty array.)
+    $plugin += array('contextual links locations' => array('view'));

That way we have contextual links enabled by default, but still give the plugin a way to disable them if they want to.

The rest of the attached patch just removes the other code that was added, since I suggested above that it's not necessary... please let me know if I'm mistaken, though.

Oh i never wanted to commit my previous patch.

Thanks!

OK, thanks!

So, the last ten comments or so were a bit of a detour, but the original issue here is still basically valid, I think? It's no longer an issue for the page display (since the contextual links are displayed in the top right corner of the page title), but since attaching the contextual links to the view is now the default behavior used e.g. by Attachment displays, it is an issue there.

In summary: If you create a view of nodes that has an attachment display, the attachment has contextual links for editing the view, but you never see them since they appear directly underneath the contextual links for editing the first node in the view.

There's no real need for attachment displays to have contextual links, is there? Maybe we should just have attachments not have them?

We could make attachments opt-out of contextual links, yes. The attached patch does that.

The bug might still occur for other (contrib) display plugins that use contextual links though, not sure.

Great.
Commited to 7.x-3.x