Closed (fixed)
Project:
Commerce Core
Version:
7.x-1.x-dev
Component:
Product
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
26 Jan 2012 at 13:03 UTC
Updated:
20 Mar 2014 at 11:11 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
rszrama commentedAs 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.
Comment #2
pounardFair enought.
Comment #3
3rdLOF commentedrszrama:
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.
Comment #4
rszrama commentedOof, 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.
Comment #5
3rdLOF commentedThanks 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.
Comment #6
rszrama commentedYou know, it's a pretty easy problem to solve in Views, so I'll see if bojanz can take a look at that tomorrow. : )
Comment #7
googletorp commentedI'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
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_linksare 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.
Comment #9
Elvar commented#7: 1417582-7-remove-contextual-dependencies.patch queued for re-testing.
Comment #11
rszrama commentedHah, 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.
Comment #12
rszrama commented#7: 1417582-7-remove-contextual-dependencies.patch queued for re-testing.
Comment #13
SeriousMatters commentedI 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.
Comment #14
rszrama commentedYep, 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
Comment #15
pounardVery nice, thanks!