WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 12 months ago

#25696 assigned defect (bug)

Double clicking update on list table inline edit removes row from dom

Reported by: mt8.biz Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Quick/Bulk Edit Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:
PR Number:

Description

Steps to reproduce:
Open "Quick Edit" -> Save the category(or tag) by double-click on save-button. ( or press twice ).

Expected behaviour:
Save the category(or tag) , and refresh the table-row.

Actual behaviour:
Save the category(or tag), But disappear the table-row.

Attachments (8)

inline-edit-tax.js.diff (763 bytes) - added by mt8.biz 6 years ago.
25573.jpg (50.6 KB) - added by mt8.biz 6 years ago.
inline.diff (938 bytes) - added by shanept 6 years ago.
Changes to inline-edit-tax.js and inline-edit-post.js.
25696.png (11.6 KB) - added by shanept 6 years ago.
edit-comments.js.diff (1.2 KB) - added by mt8.biz 6 years ago.
some fix.
25696.patch (1.1 KB) - added by afercia 4 years ago.
25696.2.patch (1.3 KB) - added by azaozz 4 years ago.
25696.3.patch (7.6 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (31)

@mt8.biz
6 years ago

#1 @knutsp
6 years ago

Confirmed for 3.8-alpha. Only happens when double clicking.

#2 follow-up: @c3mdigital
6 years ago

  • Component changed from Administration to Quick/Bulk Edit
  • Summary changed from When editing the Category (or Tag) by "Quick Edit", disappear the table-row. to Double clicking update on list table inline edit removes row from dom
  • Version changed from trunk to 3.1

I can reproduce in 3.6.1 On all List Tables that have quick edit. This is a result of how closures work in javascript and can be explained better here http://stackoverflow.com/a/10152046.

Maybe disabling the update button when the first click event fires?

#3 @helen
6 years ago

#25573 was marked as a duplicate.

#4 @helen
6 years ago

#25606 was marked as a duplicate.

#5 @helen
6 years ago

Closed #25573 (posts) and #25606 (comments), as this ticket seems to have attracted more eyes. Suggest working on a generalized solution, even if it has to go into multiple files.

Related: #25627

@shanept
6 years ago

Changes to inline-edit-tax.js and inline-edit-post.js.

@shanept
6 years ago

#6 @mt8.biz
6 years ago

Thanks For Manage Tickets. ( And Patch To English. )

Do you have problems with my patch?

#7 @mt8.biz
6 years ago

Changes to edit-comments.js .

I was referring to the patch of shanept.

Thanks.

@mt8.biz
6 years ago

some fix.

#8 @mt8.biz
6 years ago

  • Keywords needs-testing added

#9 @chriscct7
4 years ago

  • Keywords needs-refresh added

@afercia
4 years ago

#10 @afercia
4 years ago

  • Focuses ui javascript added
  • Keywords needs-refresh removed
  • Milestone changed from Awaiting Review to 4.5
  • Owner set to afercia
  • Status changed from new to assigned

I'd propose a new approach, similar to what is used in heartbeat.js (actually heartbeat.js has two ways to handle this). Basically aborting the AJAX request if any previous one has not completed yet would solve the double click (and double Enter) issue. The same approach can be used to fix #12769.

The attached patch is for testing only for now, and focuses only on inline-edit-post.js. If OK, the same thing should be done for inline-edit-tax.js. Have to check comments yet.

Any feedback would be greatly appreciated, cc @azaozz :)

Note: the simpler way to reproduce this is:

  • Quick Edit a post
  • place the cursor on any input field or textarea, focus a select or check a checkbox
  • quickly press "Enter" twice
  • after saving, the Quick Edit form closes and the table row disappears

#11 in reply to: ↑ 2 @ericlewis
4 years ago

Replying to c3mdigital:

Maybe disabling the update button when the first click event fires?

This sounds like a good approach, as this is something we already do in other cases (e.g. Publish button on the Edit Post page is disabled after click and during autosaves).

#12 follow-up: @afercia
4 years ago

Disabling buttons is problematic for keyboard users since the buttons will be focused and then disabled, causing a focus loss.

#13 in reply to: ↑ 12 ; follow-up: @ericlewis
4 years ago

Replying to afercia:

Disabling buttons is problematic for keyboard users since the buttons will be focused and then disabled, causing a focus loss.

Good to know! Should button disabling be considered an anti-pattern for that reason?

#14 in reply to: ↑ 13 @afercia
4 years ago

Replying to ericlewis:

Good to know! Should button disabling be considered an anti-pattern for that reason?

It always depends, case by case. But when disabling, hiding, removing from the DOM, a focused element then focus should be moved to the most logical place. Better: avoid a focus loss in the first place :)
Also, the Quick Edit form can be submitted pressing Enter on any field (also the textarea, select, checkboxes) so I'm not sure disabling the button would really solve the issue.

This ticket was mentioned in Slack in #core-editor by afercia. View the logs.


4 years ago

#16 @azaozz
4 years ago

25696.patch is a good start. Think we will need to abort the request when the users click Cancel so if they click on another Quick Edit it won't block it.

We also may need better "busy" indication. Clicking on another Quick Edit while the request hasn't been completed yet or had some sort of error is frustrating: nothing happens. Wondering if we should be showing wait or progress cursor?

@azaozz
4 years ago

#17 @azaozz
4 years ago

In 25696.2.patch: also add xhr.abort() when the user clicks Cancel.

This ticket was mentioned in Slack in #core-editor by azaozz. View the logs.


4 years ago

@afercia
4 years ago

#19 @afercia
4 years ago

Refreshed patch, taking care also of Terms and Comments. Some notes:

  • when saving maybe it should just return false instead of using xhr.abort()
  • should users be allowed to open a new Quick Edit form while a previous request has not completed yet? this would be easy to restore, not sure if safe
  • removed the toggle function, it was used to open the form on double click, seems no more used since [9439] and removed in [9726] unless I'm missing something
  • the parameter passed to save() and edit() is always a DOM object so calling it id is a bit confusing

Clicking on another Quick Edit while the request hasn't been completed yet or had some sort of error is frustrating: nothing happens.

Yep, see point 2. Currently, clicking to open a new form won't do anything, see below:

https://cldup.com/A5ZYGWuqWz.png

Theoretically, the whole thing could be refactored in order to allow a row to be updated "in the background" when it's closed because users clicked on a new Quick Edit link. Not sure it would be so safe and easy to implement :)

#20 @afercia
4 years ago

To recap: the patch still applies cleanly, needs a decision about this point:

should users be allowed to open a new Quick Edit form while a previous request has not completed yet?

in response to:

Clicking on another Quick Edit while the request hasn't been completed yet or had some sort of error is frustrating: nothing happens.

Note: this should fix also #12769.

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


4 years ago

#22 @chriscct7
4 years ago

  • Milestone changed from 4.5 to Future Release

#23 @afercia
12 months ago

  • Owner afercia deleted
Note: See TracTickets for help on using tickets.