Expanded contextual links look goofy when there are no items [#2047671]

Updated: Comment #N

Problem/Motivation

When a user has access to use contextual links, but not to any operations that would appear in the contextual links dropdown, the pencil "expands" to show no menu, resulting in this graphical oddity:

Steps to reproduce

Install 8.x standard.
Grant the authenticated user role permission to use contextual links.
Create a test user.
Log out and then log in as the test user.
Hover over an item that has contextual links, e.g. the (also empty and pointlessly displayed) Tools menu, or the frontpage view.
The contextual link pencil should appear. Click on it.

Proposed resolution

Do one of the following:

Hide the contextual link icon when there are no operations available to the current user.
Show the icon, but somehow indicate to the user that there are no options available, and fix the graphic.

Remaining tasks

(reviews needed, tests to be written or run, documentation to be written, etc.)

User interface changes

TBD

#914382: Contextual links incompatible with render cache
This issue was discovered when manually testing #1874640: Rename edit module to quickedit.

Comment	File	Size	Author
#11	empty-contextuals-2047671-11.patch	915 bytes	jessebeach
#11
#7	contextual_empty-2047671-2.patch	962 bytes	Wim Leers
#7
#3	2047671-n-empty-contextuals.patch	1.27 KB	jessebeach
#3
#3	sdf_sdf_sdf___d8.drupal.dev-2.png	39.68 KB	jessebeach
#3	November_2013___d8.drupal.dev_and_People___d8.drupal.dev_.png	25.94 KB	jessebeach
#2	contextual_empty-2047671-2.patch	962 bytes	Wim Leers
#2
	empty_contextual_links.png	3.78 KB	xjm

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 23 August 2013 at 11:03

Status:

Active

» Postponed (maintainer needs more info)

#2049601: Contextual links present an empty links trigger when no link items are available for an entity was marked as a duplicate of this issue.

There, it was also said that this can no longer be reproduced.

Comment #2

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 14 November 2013 at 13:39

Assigned:	Unassigned	» Wim Leers
Issue summary:	View changes
Status:	Postponed (maintainer needs more info)	» Needs review
Issue tags:		+JavaScript, +Spark, +sprint

File	Size
contextual_empty-2047671-2.patch	962 bytes

I can reproduce this using the same steps xjm provided in the issue summary, but instead of looking at the Tools menu, just look at the contextual link for the front page view.

This super simple patch fixes the problem :)

Comment #3

jessebeach CreditAttribution: jessebeach commented 15 November 2013 at 18:26

File	Size
November_2013___d8.drupal.dev_and_People___d8.drupal.dev_.png	25.94 KB
sdf_sdf_sdf___d8.drupal.dev-2.png	39.68 KB
2047671-n-empty-contextuals.patch	1.27 KB

I was able to reproduce following xjm's STR. This is what the object of contextual links looks like for an authenticated person with contextual link permission, but no editing permissions:

And this is what the contextual links object looks like for an admin user:

In both cases, a list of ids of editable items is returned. In the case that a user does not have permission to any operation that would introduce a contextual link, the content of that item is returned as an empty string.

In this case, would it not be better to exclude the id from the returned list such that the returned result set would be an empty object for an authenticated user? Here's a patch that does this.

Comment #4

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 15 November 2013 at 18:36

Good catch! :)

Unfortunately, the answer is "no, we wouldn't want that". Once #2136507: Use client-side cache tags & caching to eliminate 1 HTTP requests/page for rendering Contextual Links lands, the rendered contextual links for each ID will actually be cached on the client-side. By returning the empty string, the client-side cache knows there's nothing there for the user. If we omit the result, then the client-side cache will have to talk to the server every time, over and over again.

Comment #5

jessebeach CreditAttribution: jessebeach commented 15 November 2013 at 18:47

Status:

Needs review

» Reviewed & tested by the community

Ah, yes, I knew you'd have a clever reason. Ok, given the need to support cacheing on the client side, the patch in #2 resolves this issue. I've manually tested and there are no behavior regressions.

I am RTBCing contextual_empty-2047671-2.patch (#2)