I have created a Page view and this has added the class contextual-links-region
to the page $classes
variable and this is not the region where the hover links should be shown. I've found views_preprocess_html()
that is already killing this the CSS class contextual-links-region
attached to the body element, but the kill in the page element is missing.
Patch attached also removes the contextual-links-region
class from the 'page' classes that comes directly after the body element and currently enables *all* contextual links in the page; if you hover anywhere over the page.
Comments
Comment #1
hass CreditAttribution: hass commentedComment #2
hass CreditAttribution: hass commentedBuggy QA system
Comment #3
hass CreditAttribution: hass commentedShoot again into an empty testing queue.
Comment #4
dawehnerit would be a bit better if this comment would be "implements hook_preprocess_page".
For this you could use array_diff($variables['classes_array'], array('contextual-links-region');
Comment #5
hass CreditAttribution: hass commentedIt's 100% the same code like views_preprocess_html()... I think it makes no sense to use different logics as this may lead to unexpected failures. Otherwise we should change both functions.
Comment #6
hass CreditAttribution: hass commentedComment #7
tim.plunkettActually, it should be
Implements hook_preprocess_HOOK() for page.tpl.php.
#1443202-14: Apply documentation standards for hook_preprocess_HOOK()
Comment #8
dawehnerOkay let's fix both places.
Comment #9
hass CreditAttribution: hass commentedNew patch attached hopefully fits.
Comment #10
sunGiven the consequences of this bug outlined in the OP, this doesn't necessarily sound like a duplicate of #1170428: Contextual page links behavior causes gap above admin_menu on Seven theme.
However, since this patch attempts to remove the .contextual-links-region class from the BODY, and at the same time, refers to the JavaScript for re-injecting it, and that exact JavaScript is the cause for other bugs (as detailed in aforementioned issue), it would be good to double-check that the proposed fix here does not conflict with the necessary fix over there.
Comment #11
hass CreditAttribution: hass commented@sun: views 3.3 does not add the contextual links to body... Your JS addition is a bug that must be a leftover of one older bug. Above patch is partly just refactoring - at least in regards to body element.
Comment #12
hass CreditAttribution: hass commentedRe-attaching #9 to get the green light because of wrong version selected in past.
Comment #13
hass CreditAttribution: hass commented@dawehner: Any chance for a commit?
Comment #14
dawehnerWhat is the need to do that via array_diff, the previous code was much simpler.
Mh i'm wondering where this javascript exists at the moment.
Comment #15
hass CreditAttribution: hass commentedI changed this only for You to array_diff. See your comment #4, please or use my patch in #1.
Comment #16
hass CreditAttribution: hass commented@dawehner: What is holding this patch still back?
Comment #17
andyhu CreditAttribution: andyhu commentedtested working, hope this patch will be committed soon
Comment #18
agalitsyn CreditAttribution: agalitsyn commentedThis patch didn't help me.
Because in views-contextual.js class contextual-links-region added to body.
.contextual-links-region {
outline: none;
position: relative;
}
Position relative gives wrong intend to admin menu.
Fix see in attached patch
Comment #20
darrenmothersele CreditAttribution: darrenmothersele commentedJust fixing the formatting of patch in #18 so that it applies correctly.
Comment #21
Dmitriy.trt CreditAttribution: Dmitriy.trt commentedComment #22
knretaleato CreditAttribution: knretaleato commentedThis fixed the problem for me, hope it will be committed!
Comment #23
knretaleato CreditAttribution: knretaleato commentedOops, accidentally changed to needs work. :)
Comment #24
hass CreditAttribution: hass commentedPer feedback from several people.
Comment #25
scotwith1twould love to see this one committed too. simple enough to get around as i don't want to have to keep patching views, so i just wrote 1 line of jquery to remove it in my theme.
Comment #25.0
scotwith1tUpdated issue summary.
Comment #26
lnicks CreditAttribution: lnicks commented20: views_contextual_links_on_page_element-1493210-20.patch queued for re-testing.
Comment #27
donquixote CreditAttribution: donquixote commented#14 / #15
I agree, this looks weird.
@dawehner Any reason you asked for
array_diff()
in #4?Comment #28
BBC#20 worked for me
Comment #29
dawehnerI thought that this could replace the array_search before as well, but well, in reality it is not possible.
Let's get this in and solve it also properly in core.
Committed and pushed
Comment #32
catchComment #39
LendudeCleaning up old bugs.
As far as I can tell this has been completely refactored in D8/D9 so moving back to D7 Views and marking fixed per #29.