When you have a view the contextual links don't work anymore.

The hover itself still does work, though the contextual js kicks in too fast.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
1.09 KB

Let's move the contextual links js to hook_library_info and fix the order.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

without the patch contextual links wouldnt fire..now works as should.
also +1 to have them as library

Status: Reviewed & tested by the community » Needs work
Issue tags: -VDC

The last submitted patch, drupal-1933426-1.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#1: drupal-1933426-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-1933426-1.patch, failed testing.

ParisLiakos’s picture

Status: Needs work » Needs review
Issue tags: +VDC

#1: drupal-1933426-1.patch queued for re-testing.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

now that bot agrees

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Yay!

Committed and pushed to 8.x. Thanks!

damiankloip’s picture

Status: Fixed » Needs review

Do you think we should look at moving this into views_ui module instead? Seems it might be better living in there, as that's when we are using them.

dawehner’s picture

Not sure, as this adds support for all kind of contextual links, not just edit view, at least in theory.

damiankloip’s picture

Status: Needs review » Fixed

Hmm, true. I remember now. I guess you could use them to add filters etc.. I think we could still move all of this into the UI though?

Let's fix this again (sorry) and create a followup?

dawehner’s picture

Yeah, so views_ui is one of these modules that want to add new contextual links to it.

damiankloip’s picture

Wim Leers’s picture

If a custom weight is necessary, then it should be documented *why*. Otherwise, we end up with unintelligible loading order/dependencies.

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

Priority: Normal » Minor
Status: Closed (fixed) » Active

(Sorry for reopening this issue.)

This patch moved the .js file into a library. Yay!

But, it failed to move the weight comment along with it. Look at the patch in #1: the weight comment at the top makes no sense anymore, it should be moved into the library definition.

dawehner’s picture

Status: Active » Needs review
FileSize
526 bytes

Well, the problem is a bit tricky here.

Even this actually "depends on contextual.js" this file has to be loaded before, as it alters some css classes in order to work properly,
so what about documenting this.

xjm’s picture

Title: Fix contextual links on views » [Followup] Fix contextual links on views

Please open new issues for followups in the future. :) I had to read this whole issue to figure out it had nothing to do with what I was trying to find an issue for.

dawehner’s picture

Issue tags: -VDC

#17: drupal-1933426-17.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, drupal-1933426-17.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
529 bytes

just a reroll.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

That works, but doesn't solve the problem pointed out in #16, which is that we need to remove the comment from above the line that is now just drupal_add_library(...) without any specialness about weights.

So I went ahead and did that as well and then committed/pushed #21 to 8.x. Thanks!

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