When you create a views block, views needs to better integrate its support for the contextual links in Drupal 7.

If the block has a title that is visible on the page, then the views block will have two gear icons. One for the tile letting you edit the block and one for the view letting you edit the view.

But if you set the block title to <nonde> then you only have the contextual links to edit the view. The links to edit the block is simply gone.

Personally I would prefer only one gear icon for both of the above, containing the links to both edit the block and the view. Otherwise I believe it will be a bit confusing for some users.

Setting this to priority major since views is such an important module and being able to edit the block settings for blocks with no titles doesn't work.

CommentFileSizeAuthor
#13 views-contextual-links-followup-876772-13.patch1.9 KBDavid_Rothstein
#6 views-contextual-links-876772-6.patch23.87 KBDavid_Rothstein
#6 views-contextual-links-block.png60.79 KBDavid_Rothstein
#6 views-contextual-links-page.png53.8 KBDavid_Rothstein

Comments

dawehner’s picture

dawehner
Primary language German
commented

Views is written very abstract. I don't like the idea to hardcode something like displaying the links on every display beside blocks.

This is not flexible enough from my point of view. For example if you extend the block display you get the links again. Additional the way how the links are integrated is totally different to what core does :(

tsvenson’s picture

tsvenson commented

I'm all for flexibility, which is actually one of the main reasons I selected Drupal as my main platform to build sites on.

The problem here though is that views is changing the way D7 is handling the contextual links. I have other blocks where I have turned of the title and they still show the gear and gives a drop down menu with a link to "configure block". But if that block is a views block, then the drop down will only have links to editing the view itself, not changing the block settings.

I haven't done much module development for Drupal, but know a fair bit about PHP, etc. I took a quick look at http://api.drupal.org/api/search/7/contextual and in particular the http://api.drupal.org/api/function/hook_menu_contextual_links_alter/7 hook.

At the top of that page it says "Additional links may be added or existing links can be altered.". Forgive me a bit here, since I am treading on a bit of deep water based on my Drupal coding experience, but it seems to me that would be the right way for views to add its contextual links. It would achieve two goals in this case:

- Only one contextual gear for each views block.
- Link to edit the block settings even when no title is visible.

Additional the way how the links are integrated is totally different to what core does :(

I read that as that you have developed your own contextual link code that does not use the functions in the contextual.module at all? Is that correct?

If so, then I can understand these problems. On the other hand it makes me a bit puzzled that you did since it seems to me that using the "hook_menu_contextual_links_alter" would have achieved the same.

Views is one of the most used and important modules for Drupal. Views also have massive influence on how other module maintainers integrate their modules with Drupal. I believe it would be setting a bad example if it starts changing important UX improvements.

Viewing blocks without titles is common and by not having a link to configuring those blocks in the gear drop down will add extra work. Instead of getting the block configuration up directly the admins will have to:

- Click Structure
- Click Blocks
- Find the block in the list
- Click Configure

That is when not using other modules such as Context, Display Suite, Panels and so on that changes the way you work with blocks on the site.

Once again, sorry for barking about this, but I believe it is important that the new UX innovations for Drupal 7 is used correctly. They are there to make it easier for users and if we start to break them this early, then it only risk ending up in a mess where users get confused due to that it works different depending on what modules they are using.

dawehner’s picture

dawehner
Primary language German
commented
Priority: Major » Normal

I don't see this as a major issue. If someone want's to work on it, it's fine.

bloke_zero’s picture

bloke_zero commented

I can see it's not major but I'm with tsvenson, at times it'd be useful to be able to use hook-menu_contextual_links_alter with views for specific UX work, I came across it this morning and was surprised to see I could hook into the blocks but not the views contextual links.

For me contextual links are a big win for the overall site admin, but not so much if they are restricted.

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Closed (duplicate)

I believe http://drupal.org/node/713208 is the cause of this.

David_Rothstein’s picture

David_Rothstein commented
Title: Better contextual link integration for blocks » Better contextual link integration for blocks and pages
Status: Closed (duplicate) » Needs review
StatusFileSize
new views-contextual-links-page.png53.8 KB
new views-contextual-links-block.png60.79 KB
new views-contextual-links-876772-6.patch23.87 KB

I think this was closed by mistake? (The issue linked to above isn't related to contextual links.)

I've written a patch which reimplements Views contextual links to properly integrate them with Drupal core. As discussed above, the current Views implementation is basically duplicating the style of the core contextual links UI rather than using the system itself, which isn't good. It leads to a number of interrelated problems:

  • The contextual links on views blocks aren't properly attached to the block (but rather appear inside the block).
  • The contextual links on views pages don't appear inline with the page title like we'd expect them to (a regression compared to Drupal 6, and also causing #1024924: Contextual links inaccessible for attachment display, node style listing).
  • The views contextual links are hardcoded to work with the Views UI module, even though there can be other administrative interfaces for editing views (like SimpleViews).
  • They are also hardcoded to work with the Contextual Links module in Drupal core, even though that UI is supposed to be pluggable.

The attached patch should fix all the above issues. Here's what it does:

  • It introduces a flexible system that allows each display plugin to declare, via hook_views_plugins(), how it wants contextual links to be displayed, and then Views itself uses the core contextual links API to do the rendering. Meanwhile, administrative interfaces (like Views UI) can use hook_views_plugins_alter() to add their administrative paths to the list of contextual links.
  • The visual effect is shown in the attached screenshots. For blocks, the "Edit view" link now appears in the same dropdown as "Configure block" like it's supposed to (http://drupal.org/files/issues/views-contextual-links-block.png), while for pages, it appears next to the page title (http://drupal.org/files/issues/views-contextual-links-page.png). This should be a big improvement.
  • The patch also removes hook_views_admin_links_alter() (which should no longer be needed since the functionality for altering these links already lives in Drupal core) and some old CSS/JS for the Views admin links in Drupal 6 (which as far as I can tell, was already dead code that wasn't being used in Drupal 7).
  • The one part that is definitely hacky is the code for getting the links to appear in the correct place on page displays (i.e., for making sure the dotted outline in http://drupal.org/files/issues/views-contextual-links-page.png is drawn in the correct place). It needed some workarounds due to limitations in Drupal core, and because of that, the code winds up being semi-theme-dependent so it's not guaranteed to work well always. I'm not too happy with that. In practice, though, I tried it out on a bunch of D7 themes and it works great with all of them (with the notable exception of admin themes, such as Seven, which tend to have markup that is less well-organized for contextual links).

This patch is against the GitHub repository (https://github.com/ksenzee/views3ui) since that will soon become the new Views UI. I'll probably commit it there for testing purposes, but since it's a pretty large patch I also wanted to post it for review here so we can make sure it's the direction we want to go in and refine it if necessary...

tsvenson’s picture

tsvenson commented

Thanks David, the screenshots looks great. Brilliant work to get the contextual link to fully utilise the cure functionality instead of using duplicated module specific features.

himerus’s picture

himerus commented

subscribe, and +1

effulgentsia’s picture

effulgentsia commented

subscribe

dawehner’s picture

dawehner
Primary language German
commented

Instead of the two drupal_static's views_get_page_view might be a better alternative.

dawehner’s picture

dawehner
Primary language German
commented

Another note: the memory change in views_page could be backported, too.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

Mark as needs work based on #10
Perhaps someone will find time before myself to fix this part.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Needs review
StatusFileSize
new views-contextual-links-followup-876772-13.patch1.9 KB

Sorry, I meant to get back to this sooner.

The above patch is already merged in to 7.x-3.x, so the only thing left to do here is deal with those last remaining issues.

I was able to get it working (see attached) but in doing so I realized we also have to remove those $view->destroy() calls. The problem with those turns out to be that they make views_get_page_view() not work correctly - you wind up getting back an incomplete copy of the view.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Fixed

Code works as before.

reviewed and commited to 7.x-3.x. Thanks for all your hard work!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

bforchhammer’s picture

bforchhammer commented

Note: The Omega theme seems to be one of the incompatible themes mentioned in #6. See also: #1309660-2: Ensure $title_suffix is always rendered (i.e. fix contextual links).

Just as a reference for anyone else having problems with views' contextual links support... ;-)

c4rl’s picture

c4rl commented

As discussed above, the current Views implementation is basically duplicating the style of the core contextual links UI rather than using the system itself, which isn't good. It leads to a number of interrelated problems

Now that views is in 8.x core, it seems that we can find a more elegant solution for this, as views_preprocess_html() seems clunky. Shall we open a new issue to resolve?

effulgentsia’s picture

effulgentsia commented

Note that the block quote in #17 was referencing how Views was doing it before this issue. This issue fixed it so that much of the contextual system is used, but with a couple hacks thrown in. But I agree about needing to remove those hacks, so I opened an issue for that: #1916516: Decide whether/how to implement contextual links associated with the main page content.