.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | vdc-1758624-15.patch | 1.71 KB | tim.plunkett |
| #8 | contextual_links.patch | 6.3 KB | aspilicious |
| #1 | 1758624-move_contextual_locations-2.patch | 5.64 KB | bellesmanieres |
.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | vdc-1758624-15.patch | 1.71 KB | tim.plunkett |
| #8 | contextual_links.patch | 6.3 KB | aspilicious |
| #1 | 1758624-move_contextual_locations-2.patch | 5.64 KB | bellesmanieres |
Comments
Comment #1
bellesmanieres commentedPatch attached, the views_add_contextual_links() function will need a bit of rework after decorator as made it in, just keep it there as reminder.
Comment #2
ACF commentedComment #3
dawehnerI'm confused does the contextual links actually work for you on a clean d8 without your patch?
Comment #4
damiankloip commentedComment #6
dawehnerUpdate title so people can find it
Comment #7
aspilicious commentedHow can the last else test work?
If the first if test doesn't pass we now $plugin['contextual_links_locations'] is set. So when we enter the last else, we are trying to overwrite the 'contextual_links_locations' with 'view' what isn't going to happen. See http://stackoverflow.com/questions/2811798/php-adding-arrays-together
If we want to append view we should do: $plugin['contextual_links_locations'][] = 'view';
I'll look at this issue when this question is answered.
Comment #8
aspilicious commentedThis can't be reviewed properly due to a core bug. Attaching patch for now. So people can review the approach.
Comment #10
damiankloip commentedI know we can't test yet, but noticed these things....
Shouldn't this set this property on the plugin?
Is this what you meant to return? :)
huh?
{} ?
same here
Comment #11
aspilicious commentedThats what I did? (display plugin base)
And no it shouldn't rly set the added stuff. Happens at runtime.
Copy paste error
Thats the nasty part. There are 3 things:
1) 1 property that defines if this display plugin can have contextual links "usesContextualLinks"
2) 1 property that defines the locations: "contextualLinksLocation"
3) 1 system that allows modules to inject contextual links. Views UI has such an example
Comment #12
xjmComment #13
dawehnerSo especially point 3) of #11 seems to make this as closed (won't fix)
Comment #14
aspilicious commentedI agree
Comment #15
tim.plunkettI'm reopening this. views_add_contextual_links() makes no sense.
Comment #16
dawehnerYou american certainly have a total different definition of the "sense" :)
Comment #17
tim.plunkettI'd like to try and fix the @todos first, I was just making a note
Comment #18
wim leers+1, I think :)
Also note that the way contextual links are rendered in general (and thus also for Views' exceptional case) is being changed at #914382: Contextual links incompatible with render cache.
Comment #19
tim.plunkett#15: vdc-1758624-15.patch queued for re-testing.
Comment #20
tim.plunkettComment #21
dawehnerThis certainly needs work.
Comment #35
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #36
smustgrave commentedSince there hasn't been a follow up going to close out, but don't worry we can always re-open!
Thanks all