Make WordPress Core

Opened 15 years ago

Closed 10 years ago

#10569 closed enhancement (fixed)

Allow filtering of reply text with comment context

Reported by: aaroncampbell's profile aaroncampbell Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch commit
Focuses: accessibility Cc:

Description

I know you can specify the reply text for comments, but you can't make them specific to each comment such as "Reply to comment #1234 by Joe". This will let you do that by adding a filter called comment_reply_link_text to get_comment_reply_link() that passes along $args, $comment, and $post (just like the existing comment_reply_link filter).

I know you COULD do this with some processing on the link, but it would require pulling comment and post IDs from the URL and/or onclick, then replacing the text in the link itself. This way would be a lot simpler and only adds a single filter.

Attachments (4)

10569.001.diff (1.5 KB) - added by aaroncampbell 15 years ago.
10569.2.patch (2.7 KB) - added by joedolson 10 years ago.
Add filter over args array
10569.diff (846 bytes) - added by aaroncampbell 10 years ago.
10569.2.diff (941 bytes) - added by DrewAPicture 10 years ago.
docs tweaks

Download all attachments as: .zip

Change History (26)

#1 @azaozz
15 years ago

Perhaps we can pass $text to the existing filter making it very easy for a plugin to replace it instead of adding another filter. Would save a bit of overhead.

#2 @ryan
15 years ago

  • Milestone changed from 2.9 to Future Release

#3 @Elpie
13 years ago

See also #16433

#4 @chriscct7
10 years ago

Seems this would go hand in hand with #16433 which is slated for the 4.1 release.

#5 @chriscct7
10 years ago

  • Focuses accessibility added

Per
WCAG Checkpoint 13.1: Clearly identify the target of each link. [Priority 2]

this should be classified as focus for accessibility like was done in #16433

#6 @joedolson
10 years ago

So, I'm not seeing how this would add anything to the existing functionality. The existing comment filter passes the entire $args array into the filter, so the $argsreply_text? is already available, and there's really no reason you couldn't recreate the text link as needed.

I might be missing something, but I think that we can already do this with the existing code, and that this ticket is moot.

#7 @joedolson
10 years ago

  • Keywords 2nd-opinion added

#8 @chriscct7
10 years ago

@joedolson that was actually brought up in the initial ticket description. The idea is that it's easier to filter it before assembling the link as opposed to recreating it later (you could do something like an str_replace on the reply text as well from new to old).

#9 @joedolson
10 years ago

Yes, I guess that's some benefit; but it seems very slight. The str_replace method is what I would probably choose to do, and doesn't require any changes.

#10 @chriscct7
10 years ago

  • Keywords needs-refresh added; has-patch needs-testing removed

I agree its probably just as easy to str_replace, but it could be useful for some as its more straightforward.

Patch needs a refresh due to #16433

#11 @johnbillion
10 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Future Release to 4.1

#12 @markjaquith
10 years ago

I'm +1 on having this filter.

#13 @joedolson
10 years ago

Why not add a filter over the args array, so that we can filter any of those elements, instead?

@joedolson
10 years ago

Add filter over args array

#14 @joedolson
10 years ago

Patch includes the added reply_to_text parameter in $args, and moves the variable definition after pulling the post and comment object, so those are available to the filter.

#15 @joedolson
10 years ago

  • Keywords has-patch needs-testing dev-feedback added; needs-refresh removed

#16 @chriscct7
10 years ago

  • Keywords needs-refresh added; has-patch needs-testing dev-feedback removed

I've talked to @johnbillion here at WordCamp San Francisco 2014 and we've decided that it would be better to use an updated version of the original patch that just modifies the reply text at that particular spot.

@aaroncampbell
10 years ago

#17 @aaroncampbell
10 years ago

  • Keywords has-patch added; needs-refresh removed

Talked to @johnbillion about this and we decided to go the direction @joedolson suggested and filter the $args array. The reason is that we actually have three strings that would all be useful to filter. One displays when a user is logged out and needs to be registered, one displays to everyone else, and one is used for the aria-label. It just made more sense than adding individual filters. 10569.diff adds the new filter comment_reply_link_args

#18 @chriscct7
10 years ago

Sounds good!

@DrewAPicture
10 years ago

docs tweaks

#19 @DrewAPicture
10 years ago

  • Keywords commit added

10569.2.diff is ready to go in.

#20 follow-up: @SergeyBiryukov
10 years ago

In 10569.2.diff, seems like "Filter the text for the reply-to comment link" should be changed to "Filter the comment reply link arguments".

#21 in reply to: ↑ 20 @aaroncampbell
10 years ago

Replying to SergeyBiryukov:

In 10569.2.diff, seems like "Filter the text for the reply-to comment link" should be changed to "Filter the comment reply link arguments".

Yep, I updated the filter and didn't update the inline docs when I was working on it.

#22 @SergeyBiryukov
10 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 30039:

Add 'comment_reply_link_args' filter for get_comment_reply_link() arguments.

props joedolson, aaroncampbell, DrewAPicture.
fixes #10569.

Note: See TracTickets for help on using tickets.