Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15898 closed enhancement (fixed)

Change comment Reply to Reply and Approve if comment in moderation

Reported by: jane's profile jane Owned by: garyc40's profile garyc40
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1
Component: Comments Keywords: has-patch needs-testing needs-ui
Focuses: Cc:

Description

Current behavior: If comment is in moderation, adding a reply to the unapproved comment in the admin will approve the reply but not the comment. Submit button on reply form says "Submit Reply"

Desired behavior: If comment is already approved, submit button on reply form says "Reply" and approves the reply on submission. If comment is in moderation, adding a reply to the unapproved comment in the admin will approve the reply AND the comment. Submit button on reply form says "Approve and Reply"

UI: Add a border around original comment and reply form so it's visually clear that the two are connected.

Note: Related to http://core.trac.wordpress.org/ticket/15897

Attachments (12)

garyc40-15898.patch (3.5 KB) - added by garyc40 13 years ago.
there's a patch for that
before.png (123.6 KB) - added by garyc40 13 years ago.
how it looks BEFORE the patch is applied
after.png (98.0 KB) - added by garyc40 13 years ago.
how it looks AFTER the patch is applied
garyc40-15898-rev2.patch (4.8 KB) - added by garyc40 13 years ago.
actually approve the comment on the backend
garyc40-15898-rev3.patch (4.9 KB) - added by garyc40 13 years ago.
properly check parent comment's status before approving
15898.4.diff (10.6 KB) - added by garyc40 13 years ago.
refreshed, and properly update total item number and pagination when necessary
15898.5.diff (10.7 KB) - added by kirasong 13 years ago.
Properly removes "pending" badge at 0
15898.6.diff (10.8 KB) - added by kirasong 13 years ago.
Only Approves Comment if Reply Succeeds in being posted.
15898.7.diff (11.6 KB) - added by kirasong 13 years ago.
Toggles Reply/Approve and Reply button text when "Approve/Unapprove" links are pressed on the current post while replying.
15898-example-js.patch (2.1 KB) - added by azaozz 13 years ago.
Idea for handling the changes to the approved parrent comment
15898.8.diff (6.7 KB) - added by kirasong 13 years ago.
Refreshed and incorporated Azaozz's IRC input and suggested method of update. Also, cut out the additional UI components and pagination fixes. Will refresh again tonight to confirm proper functionality after incoming jQuery changes.
15898.9.diff (6.7 KB) - added by kirasong 13 years ago.
Fixes Approve/Unapprove links not toggling visibility after comment is approved.

Download all attachments as: .zip

Change History (31)

#1 @garyc40
13 years ago

  • Owner set to garyc40
  • Status changed from new to assigned

@garyc40
13 years ago

there's a patch for that

@garyc40
13 years ago

how it looks BEFORE the patch is applied

@garyc40
13 years ago

how it looks AFTER the patch is applied

#2 @garyc40
13 years ago

  • Keywords has-patch needs-testing needs-ui added

Patch attached.

The borders on the left and right of a comment and the reply form are actually those of the comment table (table.widefat). As a result, adding a border around the original comment and reply form requires changing the css of the whole table.

Instead, an alternative way to distinguish the original comment from the others is blurring them (See before and after).

This needs feedback from UI/UX.

@garyc40
13 years ago

actually approve the comment on the backend

@garyc40
13 years ago

properly check parent comment's status before approving

@garyc40
13 years ago

refreshed, and properly update total item number and pagination when necessary

#3 @jane
13 years ago

  • Milestone changed from Future Release to 3.2

If patch is good, this one could be considered a performance improvement (therefore eligible for 3.2) b/c it would reduce the number of actions required by user to accomplish task.

#4 @beaulebens
13 years ago

  • Cc beau@… added

#5 @markjaquith
13 years ago

Patch still applies. Works great, in my testing.

One small bug. On the Dashboard, I could get the pending comments badge to read "0" — it should go away if it is zero.

http://cl.ly/2Y1X350o3G1H1i3W0I2l/Screen_shot_2011-04-20_at_1.51.37_PM.png

Needs a JS review from one of our JS gurus.

#6 @kirasong
13 years ago

Tested, and works, but with a couple corner-case quirks

  • If you add a duplicate comment/reply, it approves the comment, then warns about the duplicate comment (reply), and hence doesn't create the reply. I presume we'd want it to not approve, if it's not going to create the reply as well?
  • If you start to fill out a reply, then click on "approve/unapprove" in the fade-in links in the comment's list entry above, the pending comments badge doesn't change, and the button still says "Approve and Reply". Do we even want the list of "fade-in" links to show when we're in the process of filling out a reply, or should they be disabled until the field is submitted/cancelled?

@kirasong
13 years ago

Properly removes "pending" badge at 0

#7 @kirasong
13 years ago

For above attachment, changed to using the same logic as the other functions that add/remove the badge within the file.

#8 @kirasong
13 years ago

  • Cc DH-Shredder added

@kirasong
13 years ago

Only Approves Comment if Reply Succeeds in being posted.

#9 @nacin
13 years ago

Cross referencing #11328, which involves double-clicking a comment to quick edit it, causing you to lose your reply. If someone can turn that patch into something worthy, that'd be great.

#10 follow-up: @nacin
13 years ago

The patches here changes "Submit Reply" to "Reply," and introduces "Approve and Reply". Originally, reply was a noun, now it's a verb. At least we're being consistent by changing "Submit Reply" to "Reply" -- but this should get UX confirmation before going in.

Per IRC discussion, if approve/unapprove is clicked during reply, then the button text should change appropriately. For a future ticket, we should probably just avoid showing action links when the reply box is open. Every other action link will kill the reply. Seems like they should cancel it first. The lack of an AYS here on a dirty form reminded me of #11328.

@kirasong
13 years ago

Toggles Reply/Approve and Reply button text when "Approve/Unapprove" links are pressed on the current post while replying.

#11 in reply to: ↑ 10 ; follow-up: @azaozz
13 years ago

Replying to nacin:

...if approve/unapprove is clicked during reply...

Originally performing any other action while the replay form is open would close it (no AYS if dirty though). Perhaps we have a small regression there.

Looking through the latest patch, it changes some stuff and adds more that doesn't seem to be needed. All that's needed seems to be to output the "Approve and Reply" instead of "Reply" in the action links and add one line in admin-ajax to handle the approving of the parent comment (possibly using another arg when parent is pending). The recalculation of pending comments and background change would be done by triggering the existing JS for "Approve".

#12 in reply to: ↑ 11 @kirasong
13 years ago

Replying to azaozz:

Originally performing any other action while the replay form is open would close it (no AYS if dirty though). Perhaps we have a small regression there.

Clicking "Approve/Unapprove" while the reply form is open on the "appearing/action links" is allowed on 3.1.1, and it doesn't close the reply form when you click on them.

As this is the case, do you mean a regression from a yet earlier WordPress version, rather than with these patches?

Looking through the latest patch, it changes some stuff and adds more that doesn't seem to be needed.<snip>

Up to this point, I'd just modified the original patch(es) to remove/repair the existing bugs.
Further feedback is, of course, appreciated, but I'll take an original look to see if/how this can be done more simply from the beginning.

#13 @scribu
13 years ago

  • Keywords 3.2-early removed

#14 follow-up: @kirasong
13 years ago

Talked about this with azaozz in IRC.
Looks like there isn't currently a clear way to call the update code directly, so going to work on a patch with a new function we can call.

The additional code (other than to allow the approval with the reply) is for three things:

1) Paginating when adding a reply (trunk currently does not change pagination on the new added replies until you refresh the page)

2) Comment count in the list (to the right of each comment, also doesn't update in trunk)

3) UI improvements, per the summary, for a border around the selected comment and reply, and fade-out of the rest of the comments.

I'm going to create the separate function, so that the code isn't duplicated like it currently is (both in the patch, and in trunk).

However, in addition, do you want me to remove the above three from the patch, to make it more focused, or are those fixes/enhancements that we do actually want in trunk?

@azaozz
13 years ago

Idea for handling the changes to the approved parrent comment

#15 in reply to: ↑ 14 @azaozz
13 years ago

Replying to DH-Shredder:

1) The comments screen is "fluid", meaning it adds more comments to the bottom as you spam/trash them. Pagination is not really a necessity in that mode.

2) See the example JS for possble solution.

3) UI improvements: not sure it's an improvement. It's quite clear to which comment the user is replying to. Of course the UI team has the final word on this.

@kirasong
13 years ago

Refreshed and incorporated Azaozz's IRC input and suggested method of update. Also, cut out the additional UI components and pagination fixes. Will refresh again tonight to confirm proper functionality after incoming jQuery changes.

#16 @kirasong
13 years ago

The patch isn't working for updating the button text on clicking "Approve/Unapprove" due to dimAfter not being called after the jQuery 1.5.2 update, per: #17309
As soon as that issue is fixed, that functionality should work properly.

#17 @kirasong
13 years ago

Due to azaozz's sweet fix in [17808] , Approve/Unapprove links now toggle the button "Reply" and "Approve and Reply" text properly.

Last edited 13 years ago by kirasong (previous) (diff)

@kirasong
13 years ago

Fixes Approve/Unapprove links not toggling visibility after comment is approved.

#18 @azaozz
13 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In [17832]:

Change comment Reply to Approve and Reply and auto-approve the comment if it is in moderation, props DH-Shredder, fixes #15898

#19 @azaozz
13 years ago

In [17983]:

Fix Reply and Approve when replying from the comment moderation page, see #15898

Note: See TracTickets for help on using tickets.