It seems odd to force this dependency, even if I'm not using it and I don't give the permission to anyone to see contextual links I would like to be able to disable it, couldn't it be a soft dependency?

Comments

rszrama’s picture

Title: Product UI depends on Contextual » Remove the Product UI dependency on Contextual
Category: bug » feature
Status: Active » Closed (works as designed)

As far as I know, all of our UI modules use some sort of contextual link. These are the links you see in the header region of a page to add new entities. You can see them wrapped in the HTML in the contextual-links-region class. So this dependency works as designed, but I don't see what the problem is for leaving it on even if you're not using it - it adds something like 150 lines of code when included, and there's no way to make it a soft dependency because of the hooks it's working through.

pounard’s picture

Fair enought.

3rdLOF’s picture

StatusFileSize
new24.49 KB

rszrama:

Actually, I think there is case for the removing the depency. For example, the situation I am on now.

The store I am developing has an element (fabric buttons) which are generated via image style --> custom view mode --> view, This element is very small (38 pixels) and thus views adds contextual link to every instance. An pain in the katoosh to theme.

As of right now, there is no way of disabling contextual links in Views via UI :http://drupal.org/node/1306564

I really need to disable them to work on the theming because they get on the way, and simply turning off contextual links temporarily would suffice. But you can't because Drupal Commerce requires, which right now is the only module that does.

UNless there is a trick somewhere to manually disable them by other means, it is a problem.

rszrama’s picture

Oof, yeah, those are in the way. However, there's still no good way for the UI to work on the back-end without them, so I can't see us ditching the dependency. The Views feature request is very reasonable, though, and in the worst case scenario you should be able to override the theme template to remove them somehow. I can't say for certain, as I haven't tried, but that's where I'd look next.

3rdLOF’s picture

Thanks rszrama. There is already a thread somewhere asking the the Views crew to allow disabling the dependendies.

I geta around it by using Masquarade and swicthing to a regular administrator role with the contextual links permission disabled.

rszrama’s picture

You know, it's a pretty easy problem to solve in Views, so I'll see if bojanz can take a look at that tomorrow. : )

googletorp’s picture

Status: Closed (works as designed) » Needs review
StatusFileSize
new1.9 KB

I'm going to make a case for removing the contextual links dependency - patch applied.

First off, I can't actually see any effect generated from the contextual links module being enabled. As far as I can see, the only place it's used is places link


/**
 * Implements hook_menu_alter().
 */
function commerce_product_ui_menu_alter(&$items) {
  // Transform the field UI tabs into contextual links.
  foreach (commerce_product_types() as $type => $product_type) {
    // Convert underscores to hyphens for the menu item argument.
    $type_arg = strtr($type, '_', '-');

    $items['admin/commerce/products/types/' . $type_arg . '/fields']['context'] = MENU_CONTEXT_PAGE | MENU_CONTEXT_INLINE;
    $items['admin/commerce/products/types/' . $type_arg . '/display']['context'] = MENU_CONTEXT_PAGE | MENU_CONTEXT_INLINE;
  }
}

I've been looking at the different pages with and without contextual links module enabled (after clearing cache) and I can't spot any difference. I might be missing something, but if I actually look for a difference and can't find it, the difference can't be important enough to actually make a difference worth creating a dependency for. I also think you are mistaken on the actual "dependency" of contextual links.

Several places

menu_contextual_links

are used, but this isn't actually a part of the contextual link module, but a part of Drupal core.

The reason I need the contextual links module disabled, is because it conflicts with with panels inline editor. It's a tool that makes it possible to edit panel pages inline, but since it also uses gears for the panel panes, it becomes a huge mess / UA fail since the users will be confused about the contextual link gears and the panel inline editing gears.

Anyways unless we can actually find a place where contextual links makes a huge difference, that ruins the UI, I really think we should remove the dependency.

Status: Needs review » Needs work

The last submitted patch, 1417582-7-remove-contextual-dependencies.patch, failed testing.

Elvar’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1417582-7-remove-contextual-dependencies.patch, failed testing.

rszrama’s picture

Version: 7.x-1.1 » 7.x-1.x-dev
Status: Needs work » Needs review

Hah, you're right of course. I don't see anything in the Contextual Links module we're using. Putting the test to 1.x-dev to get rid of the failures.

rszrama’s picture

SeriousMatters’s picture

Category: Feature request » Bug report
Issue summary: View changes

I manually applied patch / removed contextual dependencies and disabled Contextual Links module on a production site. It works just fine with no difference.

Since #7 shows that contextual dependencies were included by mistake, I'm changing this issue back to bug.

rszrama’s picture

Category: Bug report » Task
Status: Needs review » Fixed

Yep, just forgot about this. Committed. Changing to a task, as it's not like it was actually broken - just apparently unnecessary. : )

Commit: http://drupalcode.org/project/commerce.git/commitdiff/c7565f4

pounard’s picture

Very nice, thanks!

Status: Fixed » Closed (fixed)

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