I would even go as far as disabling contextual links when dashboard is in customize mode.
Not only would this remove the whole "edit-within-edit"-wtf that we have now, but I think it would also simplify the changes that need be made to hide the contextual links while dragging, as u can't drag without being in customize mode
Patch works, but it's quite rigorous yes. I'm almost convinced we should indeed kill contextual links for blocks *on* the dashboard too, but what if we'd want to add a 'remove from dashboard' contextual link to blocks shown on the dashboard?
basically that would be a "edit the region this block appears in" kind of option, right? so should we add that to all blocks at all times? (given that the user has permission to do so ofc)
yeah, true.
Also, only just realized this is indeed only while in customize mode. duh, yes lets hide them there, they interfere with the drag drop there.
I tested the patch and it does exactly what it says it does. I agree with the others here that the existing behavior is confusing. Let users focus on positing the blocks.
Whoops, accidentally uploaded a modified version of the patch that removes "div" from the selectors as they are no needed. I noticed that all of the other contextual selectors use div so this change is not appropriate here. The patch in #5 is what should be committed. I applied it without problem today on a fresh copy of head.
Bojhan, it is not about improving the experience -- it is about fixing a bug.
I don't think this is the proper bugfix though. Try with the 'powered by Drupal' block and you see that we loose the title. In other words, it is not just the contextual links that get messed up.
I am having real trouble understanding what this issue is about.
The initial video that was posted to start this issue is definitely a bug - it shows contextual links appearing on disabled blocks within the dashboard. That we definitely don't want. Is that what this issue is about? If so, I just filed a duplicate bug here, it seems: #831088: Contextual links are showing for disabled dashboard blocks
However, the patch here instead seems like it is hiding the contextual links for the enabled dashboard blocks while the dashboard is being configured. In that case, I think Bojhan is correct - why would we ever want to do that? The current behavior is not a bug but rather is intentional as per #639162: Block contextual menu should be available immediately when dragged to dashboard canvas area and makes plenty of sense. When I click on a "configure dashboard" link I expect to be able to, um, configure the dashboard, which doesn't just mean moving blocks around but rather might also involve fiddling with their titles, etc.
Meanwhile, the issue Dries raised in #16 about the Powered by Drupal block seems like a totally independent bug which should be filed elsewhere. I don't see how it's caused by this patch.
Now that there's a little gear for contextual links that does not interfere with anything here, I don't think the original issue here is necessarily valid (the contextual links are fine to have on the dashboard)
The other issues mentioned here have other tickets open.
Comments
#1
tag
#2
I would even go as far as disabling contextual links when dashboard is in customize mode.
Not only would this remove the whole "edit-within-edit"-wtf that we have now, but I think it would also simplify the changes that need be made to hide the contextual links while dragging, as u can't drag without being in customize mode
#3
attached patch adds 2 CSS rules that hide the trigger and disable the outline when the contextual links are a child of div.customize_mode
I figured contextual.css was the best place to handle this
#658034: Core should not use CSS classnames that contain an underscore will break this and vice versa
#4
#658034: Core should not use CSS classnames that contain an underscore was committed
#5
rerolled relative to div.customize-mode
#6
Patch works, but it's quite rigorous yes. I'm almost convinced we should indeed kill contextual links for blocks *on* the dashboard too, but what if we'd want to add a 'remove from dashboard' contextual link to blocks shown on the dashboard?
#7
basically that would be a "edit the region this block appears in" kind of option, right? so should we add that to all blocks at all times? (given that the user has permission to do so ofc)
#8
yeah, true.
Also, only just realized this is indeed only while in customize mode. duh, yes lets hide them there, they interfere with the drag drop there.
edit: patch does indeed still apply etc.
#9
Re-test of 639172-disable-contextual-5.patch from comment #5 was requested by webchick.
#10
Re-test of 639172-disable-contextual-5.patch from comment #5 was requested by yoroy.
#11
Wait why would we disable contextual links in customize mode? Isn't that the one time, you are most likely to use this?
#12
I tested the patch and it does exactly what it says it does. I agree with the others here that the existing behavior is confusing. Let users focus on positing the blocks.
#13
Whoops, accidentally uploaded a modified version of the patch that removes "div" from the selectors as they are no needed. I noticed that all of the other contextual selectors use div so this change is not appropriate here. The patch in #5 is what should be committed. I applied it without problem today on a fresh copy of head.
#14
still applies and does the trick.
#15
No idea why we would limit this? What is the desired experience win from this?
#16
Bojhan, it is not about improving the experience -- it is about fixing a bug.
I don't think this is the proper bugfix though. Try with the 'powered by Drupal' block and you see that we loose the title. In other words, it is not just the contextual links that get messed up.
#17
I am having real trouble understanding what this issue is about.
Clear as mud, right? :)
#18
well then we should at least restyle it, coz the outline in the outline looks mighty funky
#19
Now that there's a little gear for contextual links that does not interfere with anything here, I don't think the original issue here is necessarily valid (the contextual links are fine to have on the dashboard)
The other issues mentioned here have other tickets open.