Make WordPress Core

Opened 10 years ago

Last modified 14 months ago

#28326 new defect (bug)

List Tables don't update properly after Quick Edit

Reported by: wonderboymusic's profile wonderboymusic Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Quick/Bulk Edit Keywords: bulk-reopened has-patch needs-testing 2nd-opinion
Focuses: javascript, administration Cc:

Description

Scenario: I have 2 sticky posts. I go to the Post list table and click the "Sticky (2)" link.

I see my 2 stickies.
I Quick Edit one of them and make it no sticky.

What do I see? (drumroll):
Both posts + a link that says "Sticky (2)"

Attachments (3)

Screen Shot 2014-05-21 at 10.48.33 AM.png (44.9 KB) - added by wonderboymusic 10 years ago.
After I remove stickiness from one of them
28326.diff (2.7 KB) - added by wonderboymusic 10 years ago.
28326.2.diff (2.4 KB) - added by garrett-eclipse 5 years ago.
Initial patch to update sticky counts using similar functions as in comment-reply.js

Download all attachments as: .zip

Change History (14)

@wonderboymusic
10 years ago

After I remove stickiness from one of them

#1 @wonderboymusic
10 years ago

  • Keywords has-patch added; needs-patch removed

28326.diff fixes this. The list table JS feels like it is from the Dark Ages, hard to fidget with.

#2 @wonderboymusic
10 years ago

  • Summary changed from List Table for Stickies doesn't update properly after Quick Edit to List Tables don't update properly after Quick Edit

I have changed my mind about this - I think that some Quick Edit actions should tell the page to reload. This would also fix the issue with transition post status

#3 @wonderboymusic
10 years ago

  • Keywords needs-refresh added

#4 @wonderboymusic
10 years ago

  • Keywords needs-patch added; has-patch needs-refresh removed
  • Milestone changed from 4.0 to Future Release

I don't really have the energy for this right now. My patch is a super-hack.

#5 @helen
10 years ago

Probably would rather spend energy on #28050, anyway.

#6 @chriscct7
8 years ago

@wonderboymusic want to tackle this again?

@garrett-eclipse
5 years ago

Initial patch to update sticky counts using similar functions as in comment-reply.js

#9 @garrett-eclipse
5 years ago

  • Keywords bulk-reopened has-patch added; needs-patch removed

Hi @wonderboymusic thanks for the initial work on this.

I came across this while looking at #46017 which is to update the 'Mine' comment count. Looking at the implementation there I was able to put an initial patch together 28326.2.diff to update the sticky counts.

Reusing the getCount, updateCount and updateCountText functions from edit-comments.js with a few modifications this was fairly straight-forward. I feel it would be worth looking into making this consistent across the List Tables as there's several needs to update counts and a common function would make sense. Moving these to common.js and making the markup consistent across list tables would be the next step here.

Concerning the idea of removing the sticky item from the list. I personally warn against removing the items from the current list view if enacted upon via Quick Edit. Simply removing the 'Sticky' label in my eyes would suffice, and would allow the user to undo their change. Removing the item from the list forcing the user to find it again if they unstuck in in error.
*In a large publication with hundreds of posts and numerous featured (sticky) accidentally unsticking an item could be tragic and hard to reverse.

#10 @birgire
5 years ago

Thanks for the patch @garrett-eclipse

I tested 28326.2.diff for few cases:

1) No existing sticky posts.

  • Table nav contains no Sticky link
  • Quick edit same post.
  • Mark it as sticky.
  • Press Update.
  • We don't get Sticky (1) in table's nav as expected.

2) One existing sticky post.

  • Table nav contains Sticky (1) link
  • Quick edit same post.
  • Mark it as non-sticky.
  • Press Update.
  • We get Sticky (0) in table's nav (Should it be removed?).
  • We have to refresh page to remove Sticky (0) in table's nav.

3) One existing sticky post.

  • Table nav contains Sticky (1) link
  • Quick edit another post.
  • Mark it as sticky.
  • Press Update.
  • We get Sticky (2) in table's nav, as expected.
  • When we refresh page we also get Sticky (2) in table's nav.

In 28326.2.diff I noticed the debugging parts of:

console.log(rowData);
console.log(was_sticky);
console.log(r);
console.log(is_sticky);

For consistency with camelCase, I would suggest wasSticky and isSticky variable names,
instead of was_sticky and is_sticky.

Then the docblocks could be added for the three methods.

I first wondered about the difference between was- and is- sticky formulation:

was_sticky = $( '.sticky', rowData ).text() === 'sticky';
is_sticky = -1 !== r.indexOf( '<div class="sticky">sticky</div>' );

but this is because r (the ajax post response) is a string but rowData is a jQuery instance.

I wonder if it could use:

is_sticky = $( '.sticky', r ).text() === 'sticky';

to skip the exact HTML code reference and to make the is- and was- checks similar? That seems to be working from quick testing.

Last edited 5 years ago by birgire (previous) (diff)

#11 @webcommsat
15 months ago

  • Keywords needs-testing added

Component review today
Checked against other open tickets for similarity.
This needs testing to check if still current.
Tests to take place in the next two weeks and @oglekler and @webcommsat to have this ticket available at WordCamp Asia Contributor Day for anyone who would like to test it.

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


15 months ago

#13 @oglekler
14 months ago

  • Keywords 2nd-opinion added

We have similar ticket #41494 closed with status maybelater. It looks like this inconsistency with posts counts needs to be addressed because users expecting to see it, but the solution needs to be complex and rebuild the whole menu and not only making update of sticky posts. Possibly it can be return back via ajax request. Possible and easiest ways to make this needs to be discussed.

Note: See TracTickets for help on using tickets.