Menus screen: fix persistence of toggles for displayed nav menu item properties

[afercia]

Splitting this out from #35273.

The Screen Options in nav-menus.php would need a similar treatment as the one used for the options in the Customizer.

When quickly clicking on the checkboxes, multiple separate AJAX requests fire and there's no guarantee about the order they will be processed. This could lead to a non-synchronized state of the options in the Menus Screen and in the Customizer.

To consider: maybe do this for all the screen options in all the admin screens?

For details, see #35273 and the solution implemented in [36908].

[ryankienstra]

Pull Request

Please see this ​GitHub pull request for the code review of this ticket's code.

[westonruter]

@ryankienstra I found one more edge case related to our PR: ​https://github.com/xwp/wordpress-develop/pull/159

See screen options.png​. The screen options on the edit posts screen, in addition to these checkboxes, also has a form for inputting pagination and the view mode. This form has an Apply button. If you are quick with your checkbox selections and then quickly tab down to hit Apply, the hidden-columns Ajax request will not have a change to send, and once the form has been submitted and the page reloads, the checkboxes will remain as they were before.

The only thing I can think of to fix this would be to perhaps add a submit event handler for the adv-settings form which can listen to whether or not there is a pending hidden-columns Ajax request. If so, then the submit should be prevented and instead deferred until the Ajax request is done. Not super pretty. There may be a better way to go about it, like maybe including the hidden-columns in the adv-settings request.

[ryankienstra]

Commits To Address Form Submission Issue

Hi @westonruter,
You commented above that on edit.php page, it's possible to refresh the page by clicking "Apply," before the ajax requests are sent. This ​commit to this ticket's ​GitHub pull request passes ​true as the immediate argument to _.debounce. So this submits the first ajax request, without a 2000 ms delay. This fixes an issue where you click one "Columns" checkbox, like "Author," and quickly click "Apply."

But there's still an issue if you click on several screen options, and then click "Apply." This next ​commit delays form submission until the current ajax request completes. But there could be 3 ajax requests queued with ​_.debounce. This handler isn't very useful for clicking multiple screen options, and clicking "Apply."

[ryankienstra]

Request For Review Of Patch

Hi @afercia,
Could you please review this patch, which debounces the ajax requests on selecting screen options? Most of the code review was on the ​GitHub pull request, which is identical to the attached patch.

[afercia]

Sorry for the late reply. Tested 36202.diff​ and seems to me it works just on the menus screen and just for the "Boxes" checkboxes, not for the "Show advanced menu properties" ones.
On edit.php doesn't work as intended since, as you pointed out, immediate is set to true so on a first click the request is triggered immediately and any subsequent click in the following 2 seconds doesn't trigger a new request: nothing happens. You have to wait two seconds to actually trigger a new requests. The columns disappear just because the hidden CSS class gets applied but, for example, when quickly unchecking the columns, wait a few seconds, and then refresh the page, their state is not saved. Just the first one is saved.
Preventing the form submit when clicking "Apply" is not so ideal, I think, since it could lead to unexpected results and mislead users. They could think everything is saved, while it is not.

Overall, I think we're trying to solve with JS an issue that it's intrinsic to the UI. Maybe we should reconsider the reasons why the settings tab is using two different save methods in the same UI. I'd say if they work in a different way, that should be self-evident in the UI itself. I'd also consider to entirely remove the AJAX saving as pointed out by @helen in a similar case on the Customizer.

[afercia]

Discussed in today's dev bug-scrub and since time is running out for this release cycle, punting. I'd also recommend to do an effort to think at this globally, since there are other parts of the UI in the admin with the same issue.