WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 6 months ago

#28326 new defect (bug)

List Tables don't update properly after Quick Edit

Reported by: wonderboymusic Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Quick/Bulk Edit Keywords: bulk-reopened has-patch
Focuses: javascript, administration Cc:
PR Number:

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 6 years ago.
After I remove stickiness from one of them
28326.diff (2.7 KB) - added by wonderboymusic 6 years ago.
28326.2.diff (2.4 KB) - added by garrett-eclipse 8 months ago.
Initial patch to update sticky counts using similar functions as in comment-reply.js

Download all attachments as: .zip

Change History (11)

@wonderboymusic
6 years ago

After I remove stickiness from one of them

@wonderboymusic
6 years ago

#1 @wonderboymusic
6 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
6 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
6 years ago

  • Keywords needs-refresh added

#4 @wonderboymusic
5 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
5 years ago

Probably would rather spend energy on #28050, anyway.

#6 @chriscct7
4 years ago

@wonderboymusic want to tackle this again?

@garrett-eclipse
8 months ago

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

#9 @garrett-eclipse
8 months 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
8 months 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 8 months ago by birgire (previous) (diff)
Note: See TracTickets for help on using tickets.