Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Followup from #607244: Decrease performance impact of contextual links. Edit and delete are duplicates, approve just shouldn't be there unless the comment is unpublished.
Comment | File | Size | Author |
---|---|---|---|
#6 | comment_links.patch | 1.64 KB | catch |
#3 | comment_links_before.png | 16.03 KB | catch |
#3 | comment_links_after.png | 13.79 KB | catch |
#2 | comment_approve.patch | 2.21 KB | catch |
comment_approve.patch | 2.21 KB | catch | |
Comments
Comment #2
catchdoh.
Comment #3
catchScreenshots:
Comment #4
David_Rothstein CreditAttribution: David_Rothstein commentedI'm intending to add some more detailed comments to #607244: Decrease performance impact of contextual links about this later today, but it's not entirely clear that comment links should be contextual links at all (for both performance and usability reasons). They kind of seem to have slipped into the contextual-links patch at the last minute, but IMO shouldn't have because comments are a special case and really need their own issue.
So the best way to fix this might turn out to be different than the patch here...
Comment #5
Bojhan CreditAttribution: Bojhan commentedThis seems somewhat bigger implications than this patch implies. Because how will we handle, report spam links? How will a themer work on that, does he have to diffrentiate between diffrent permission action links?
Comment #6
catchSpoke to Bojhan in irc. For now, we should just rollback contextual links for comments. Then we can add them back once this usability issue, and the performance issue over at #611590: Statically cache comments are both resolved.
Comment #7
catchComment #8
Bojhan CreditAttribution: Bojhan commentedSeems good to me, I don't see us adding contextual links to comments soon - since we have many unresolved UX issues. I hope tha_sun can chime in, because I thought he removed it, and also put it in.
Comment #9
Dries CreditAttribution: Dries commentedBefore rolling this back, I'd like to discuss this some more in #607244: Decrease performance impact of contextual links. Still waiting for feedback on my last comment there. I'm not against rolling this back if necessary, but we don't need to rush it. Let's discuss it first.
Comment #10
sun.
Comment #11
Crell CreditAttribution: Crell commented@Dries: There's another problem with removing the $links versions: From a theming perspective, contextual links may not always be desirable. If they're not available, we need the functionality to still be available in the current mechanism. See: #616618: Allow contextual links to be disabled
IMO it's way too soon to move critical functionality to what is a brand new and untested UI concept, especially this late in the dev cycle. Let me mature first and see how much of an impact it has on theming and other workflows.
Comment #12
sun#607244: Decrease performance impact of contextual links, thanks.