Edit should use the new drupalContextualLinkAdded event to cleanly instead of hackily insert its "Quick edit" contextual link [#1981760]

Title says it all.

That event is being introduced at #1971108: Convert contextual.js to use Backbone (and support dynamic contextual links).

From #2 of that issue:

Essentially, I want to get rid of this hacky, ad-hoc solution:

   // Add "Quick edit" links to all contextual menus where editing the full
   // node is possible.
   // @todo Generalize this to work for all entities.
-  $('ul.contextual-links li.node-edit')
+  $('ul.contextual-links').find('li.node-edit, li.taxonomy-edit, li.custom-block-edit')
   .before('<li class="quick-edit"></li>')

This will become more important once #1966704: In-place editing for taxonomy terms and custom blocks lands.

Comment	File	Size	Author
#6	core-js-edit-dynamic-contextual-1981760-6.patch	10.62 KB	nod_
#6	interdiff.txt	5.39 KB	nod_
#5	edit_dynamic_contextual_link-1981760-5.patch	6.41 KB	wim leers
#5	interdiff-review.txt	2.49 KB	wim leers
#5	interdiff-touch.txt	806 bytes	wim leers
#1	edit_dynamic_contextual_link-1981760-1.patch	6.06 KB	wim leers

Comments

Comment #1

wim leers

Ghent 🇧🇪🇪🇺

commented 28 April 2013 at 15:48

Status	File	Size
new	edit_dynamic_contextual_link-1981760-1.patch	6.06 KB

Not NR because #1971108: Convert contextual.js to use Backbone (and support dynamic contextual links) needs to land first.

Comment #2

wim leers

Ghent 🇧🇪🇪🇺

commented 2 May 2013 at 07:50

Status:

Postponed

» Needs review

#1971108: Convert contextual.js to use Backbone (and support dynamic contextual links) landed, now this is unblocked. This patch should still be good to go.

Comment #3

wim leers

Ghent 🇧🇪🇪🇺

commented 2 May 2013 at 08:22

Manually tested on simplytest.me, still works fine.

Comment #4

nod_

French

Lille

commented 2 May 2013 at 08:37

Status:

Needs review

» Needs work

Drupal.edit = Drupal.edit || { metadataCache: {}, contextualLinksQueue: [] }; can be Drupal.edit = { metadataCache: {}, contextualLinksQueue: [] };
var that = this; not needed.
var hasFieldWithPermission = function(editIDs) { to function hasFieldWithPermission (editIDs) {
Same for allMetadataExists
var app = new Drupal.edit.EditAppView({ can be Drupal.edit.app = new …

And we probably want to put the model/views definition and instances at the same places as in contextual.js

Comment #5

wim leers

Ghent 🇧🇪🇪🇺

commented 3 May 2013 at 10:33

Status:

Needs work

» Needs review

Status	File	Size
new	interdiff-touch.txt	806 bytes
new	interdiff-review.txt	2.49 KB
new	edit_dynamic_contextual_link-1981760-5.patch	6.41 KB

#4: Thanks, all fixed! With the exception of "model/views in same places as contextual.js", because that would break #1979784: Factor Create.js and VIE.js out of the Edit module very significantly, I'll get that done there instead. That's a refactoring issue anyway, so it makes a lot more sense to do it there :) I promise it'll get done.
See interdiff-review.txt.

I made one other change: touch support. That was added to #1971108: Convert contextual.js to use Backbone (and support dynamic contextual links) after this issue was split off from it.
See interdiff-touch.txt.